Skip to content
This repository has been archived by the owner on Apr 11, 2023. It is now read-only.

Fix opaque console iframe overlaying some websites #1437

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jessarcher
Copy link

@jessarcher jessarcher commented Jul 6, 2022

This PR fixes the following issue where the console iframe is not transparent on sites that specify a color-scheme, such as GitHub:

image

The issue is caused by a change in Firefox where it will now make iframes opaque if their color-scheme differs from the parent page. The Vim Vixen iframe currently doesn't specify a color-scheme, but sites like GitHub do, hence the issue.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1738380 and https://fvsch.com/transparent-iframes

This PR makes two changes to solve this:

1. Set the appropriate color-scheme on the console.

The ConsoleFramePresenter gets the computed color-scheme for the <body> tag where the <iframe> will be injected and passes it to the console page via a query string parameter.

The console page takes the query string parameter and updates the colorScheme style property on the root element.

While this does fix the issue once the page has loaded, the white overlay is still briefly visible while it's loading.

I tried several methods to set the color-scheme but this was the only way I could set the value with the cross-origin of the <iframe>. I'm open to alternatives.

2. Shrink the <iframe> when not in use.

To hide the white overlay while the page is loading, I've set the initial height of the <iframe> to 0px. The useAutoResize hook already sets the correct height on the <iframe> once the user triggers it.

While not strictly necessary, I've also set the <iframe> height back to 0px when the console is closed. This solves a minor issue I've had during web development where the browser element selector will highlight the invisible Vim Vixen <iframe> at the bottom of the page which makes it hard to select items underneath it.

The shrinking of the <iframe> makes setting the color-scheme almost unnecessary. However, I've left it for a few reasons:

  1. When closing the console, occasionally I was able to see the white background briefly before the resize kicks in.
  2. When the console is open, there is a thin white line visible above it. This could probably be solved with CSS.
  3. Making the background transparent is presumably valuable for those with a semi-opaque console background theme.

Fixes #1424
Fixes #1429
Fixes #1433

Comment on lines 9 to 10
/* @ts-ignore */
window.document.documentElement.style.colorScheme = colorScheme
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The colorScheme property is not on the CSSStyleDeclaration type in the project's TypeScript version, which is currently pinned to 4.3.5.

We can remove the @ts-ignore by upgrading to TypeScript 4.4+, but I wasn't sure if there was a specific reason that it was pinned to 4.3.5.

@jessarcher
Copy link
Author

jessarcher commented Jul 6, 2022

For anyone that wants to try this, you can build my branch as follows. You will need Git, Node, Yarn, and a Firefox edition that supports unsigned addons. Make sure you understand the risks before continuing.

git clone git@github.com:jessarcher/vim-vixen.git
cd vim-vixen
git checkout fix-console-bg
yarn install
yarn package

This should create a vim-vixen-1.2.4.zip file in this directory which you can install from about:addons.

Again, only do this if you understand and accept the risks of enabling unsigned addons and always compile from source!

@ditsuke
Copy link

ditsuke commented Jul 8, 2022

@ueokande I'm also considering opening a PR for #111. Can you let me know if you're welcoming PRs here?

@danjones1618
Copy link

Just installed this patch and it seems to be working. Thanks @jessarcher

@jsoo1
Copy link

jsoo1 commented Jul 11, 2022

Also confirm this looks good. I don't really see any other means of communicating the color-scheme to the iframe, either. This LGTM.

@cdbrkfxrpt
Copy link

Installed and works as intended. Thanks for the fix.

@LordPax
Copy link

LordPax commented Jul 18, 2022

i have a question, when this PR will be merge ?

@johalun
Copy link

johalun commented Jul 21, 2022

How do you install the add-on? I've disabled xpinstall.signatures.required but still getting not verified error. On latest macOS and Firefox 102.0.1.

@LordPax
Copy link

LordPax commented Jul 21, 2022

@johalun it's because you dont use developer version of firefox

@danjones1618
Copy link

You can migrate to the dev version using your normal FF profile by doing firefox -p

@johalun
Copy link

johalun commented Jul 21, 2022

Thanks for the clarification. I don't wanna mess anything up so I'll just wait for this to be merged.

@xeruf
Copy link

xeruf commented Jul 22, 2022

The developer of this plugin seems to be inactive :/ maybe we need a community fork?

@jman-schief
Copy link

To all that complain: can we please cut some slack to the project maintainer(s)? Being passionate about this project does not mean that entitled comments are fine either.

There are some workarounds explained in the comments, there is a patch ready for review that can be applied with some legwork. There are even alternatives to this extensions.

This is a FOSS project I've been happily using for free out of the goodwill of the maintainer(s) and by no means I expect them to service me like I'm a paying customer.

@applejag
Copy link

The developer of this plugin seems to be inactive :/ maybe we need a community fork?

Too soon for calling the repo dead. My guess is that they're prolly on vacation. It is the middle of summer after all.

@xeruf
Copy link

xeruf commented Jul 22, 2022

either way it would be good to have co-maintainers for a project as big and good as this ;)

@johalun
Copy link

johalun commented Jul 22, 2022

To all that complain: ...

I don't see any complaints, rather suggestions on how to avoid disruptions in development. There's nothing wrong with having multiple maintainers for a project that many depends on in their daily life/work. It would also take stress and burden off the main developer.

src/console/index.tsx Outdated Show resolved Hide resolved
Co-authored-by: Benny Powers <bennypowers@users.noreply.github.com>
@fosskers
Copy link

fosskers commented Sep 20, 2022

We might be at the stage where a fork is necessary.

@codeDude64
Copy link

Hey @fosskers is there any news about What's happened with the PR merge status or about the fork that you mentioned?
It's pretty strange that the solution is there but nobody merged it

@fosskers
Copy link

fosskers commented Oct 13, 2022

At this point someone will have to volunteer to manage a fork of the project. I just started a new job and have many other FOSS projects, so it would be irresponsible of me to personally volunteer.

@nattyg93
Copy link

nattyg93 commented Oct 13, 2022

I suspect part of the problem is that no single person wants to put their hand up to take on the project. As demonstrated by the comments on this PR, we clearly have an engaged community using this fantastic extension, and many hands make light work, as the expression goes.

Are there people here that would be willing to help co-manage a fork?

Edit: Add a thumbs up here if you'd like to join the wonderful team of volunteers. 🙌

@codeDude64
Copy link

@nattyg93 I've been a developer for a lot of time but I've never participated in an open-source project, I think that It's a good opportunity for me. I'm in

@nattyg93
Copy link

@LordPax I'll take that thumbs up react to mean you're volunteering?

I'm in much the same position as @codeDude64 - I've never participated in an open-source project (other than contributing an occasional PR). It would be fantastic to have a volunteer with some experience working on a team to manage an open-source project. Any takers?

@victorz
Copy link

victorz commented Oct 13, 2022

Professional front-end developer for over 5 years now, love open source, I could help with some code review or something if you need me? Sign me up.

@LordPax
Copy link

LordPax commented Oct 13, 2022

@nattyg93 yes i'm volunteer, but my only experience is some PR on little repositories

@nattyg93
Copy link

nattyg93 commented Oct 13, 2022

@codeDude64 @LordPax @victorz I've created the organisation vim-vix-remix, forked the project there and invited you all. I've got to get to my day job, so I won't be able to do much more before Sunday.

@jessarcher Would you mind bringing this PR across to vim-vix-remix/vim-vix-remix?

Also, I'm new to all of this, so my apologies if I get any of this wrong - please let me know if there is a better/proper way of doing something. 🙂

@jman-schief
Copy link

jman-schief commented Oct 14, 2022

@nattyg93 (and all volunteers): I appreciate your intent to move this project forward with this patch, I am unclear on the goal of the fork.

Do you want to:

a) just package a fork of VIM Vixen with this patch included? As suggested earlier in this comment (and following), a way out would be to repackage the fork from the contributor repo and distribute an unsigned version, which requires some fiddling in Firefox (but I believe it is doable?).

or:

b) publish a fork of VIM Vixen with this patch for everyone on addon.mozilla.org? That procedure needs the following:

  1. a verified Mozilla AMO account (which a trusted party should open and be responsible for)
  2. building and signing the new addon for publishing under a new name (that will be a completely new addon in the store - please be aware that this might create confusion in users)
  3. satisfy the reviewing process (usually not cumbersome) and have the addon finally published

This is not to discourage anyone, just raising awareness about the work and committment that each option requires.

@nattyg93
Copy link

@jman-schief Thanks for bringing attention to the commitment and effort required for option b - which is what we are looking to do.

I'm not fully across everything that goes into getting an addon into the Firefox store, but had assumed it would be somewhat along the lines of the procedure you have outlined. It sounds like you have some experience in this area and it would be fantastic and highly appreciated if you'd be willing and able to offer some assistance. 🙂

@codeDude64
Copy link

I think we need some platform to communicate us, because I don't know What next? 😂

@jman-schief
Copy link

@nattyg93 yeah you got me, I have experience with publishing on AMO :) I have quite a lot on my plate but I'll try to help if needed.

Agree on moving further discussion away from this PR (to avoid unnecessary noise). One idea could be using the same Matrix channel that Vim Vixen is using (see the README) as long this is not intended as a takeover of the channel project, but rather as a will to keep all communication channels open with the original project maintainer. Or maybe another new Matrix channel. My personal wish would be to avoid proprietary platforms (slack, discord, etc.)

@victorz
Copy link

victorz commented Oct 22, 2022

@nattyg93, apologies, Nat, I managed to miss the invite and it expired. Would you mind inviting me again? Thanks, and sorry about that!

@kotval
Copy link

kotval commented Dec 7, 2022

@nattyg93 What matrix channel are you guys continuing on? I can't seem to find anything. Other than the new repo which hasn't had this changed merged in yet? I am willing to help.

@fpf3
Copy link

fpf3 commented Dec 31, 2022

For the lazy, I have packaged jess arch's branch into an unlisted firefox extension:

https://addons.mozilla.org/firefox/downloads/file/4050499/c8d6b82b1f8a4ef981e4-0.0.1.xpi

Hopefully this plays well with account sync, etc. If you don't trust my build, you can do the same just by following Jess' build instructions above, and following the steps on this page:

https://extensionworkshop.com/documentation/publish/submitting-an-add-on/

Make sure to choose "On your own" when it asks you how you want to distribute it. That way it will get approved in minutes, and it won't be listed publicly.

Hope this helps.

@weilbith
Copy link

weilbith commented Jan 2, 2023

For the lazy, I have packaged jess arch's branch into an unlisted firefox extension:

https://addons.mozilla.org/firefox/downloads/file/4050499/c8d6b82b1f8a4ef981e4-0.0.1.xpi

@fpf3
According to curl an my Firefox network tools, this returns a 404 status code. What is wrong?

@fpf3
Copy link

fpf3 commented Jan 3, 2023

@weilbith Not sure... I can't get at it either. I will try to figure out what's going on later tonight.

@fpf3
Copy link

fpf3 commented Jan 3, 2023

fpf3_vim_vixen_dark_0.0.1.zip

Here, I just uploaded it straight to Github. Definitely doesn't play nice with sync, but better than nothing.

@weilbith
Copy link

weilbith commented Jan 3, 2023

Fu** that is so awesome to have it working properly again. 😍
Thank you very much @fpf3

@johalun
Copy link

johalun commented Jan 3, 2023

Thank you!!

@fpf3
Copy link

fpf3 commented Jan 3, 2023

Fu** that is so awesome to have it working properly again. 😍 Thank you very much @fpf3

You're welcome, but thank @jessarcher ! It's her PR.

Anyway, I hope @ueokande is alive and this will be merged someday. This extension is a huge productivity saver for me, so if it ceases to be maintained I'm gonna be upset :)

@jiriks74
Copy link

jiriks74 commented Jan 4, 2023

fpf3_vim_vixen_dark_0.0.1.zip

Here, I just uploaded it straight to Github. Definitely doesn't play nice with sync, but better than nothing.

Thanks so, so much for this!
I used Tridactyl for some time. While it has some interesting functionality, I hated the keybinds. Mainly the search. Doing CTRL+G to next search and the fact that I'm used to enter after I type the search string was annoying as hell (btw, if it found link and you pressed enter it opened it). I love that I can have it working again! Nice work!

Hopefully someone skilled and driven enough will be able to take this on, as this is, like others said already, the best vim keybinds addon out there. I'd love to help/take it on, but I'm still a dumb beginner in programming so, I'd probably mess it up to an unworkable state LOL. Maybe some time in the future!

Again, thanks for the PR @jessarcher!

@codeDude64
Copy link

fpf3_vim_vixen_dark_0.0.1.zip

Here, I just uploaded it straight to Github. Definitely doesn't play nice with sync, but better than nothing.

I don't get it. What Do I need to do with the zip file?
When I follow the instructions of https://extensionworkshop.com/documentation/publish/submitting-an-add-on/
The platform trows an error about the manifest.json

LordPax added a commit to vim-vix-remix/vim-vix-remix that referenced this pull request Jan 6, 2023
@fpf3
Copy link

fpf3 commented Jan 8, 2023

@codeDude64 Unzip the file and open the xpi in firefox. It will install automatically.

@nattyg93
Copy link

nattyg93 commented Jan 30, 2023

@kotval (and everyone else) - sorry for the late replay, I have started a discussion over on the fork. Please, feel free to start adding to the conversation there. 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet