fx-private-relay-add-on
fx-private-relay-add-on copied to clipboard
High z-index value for email field wrapper causes stacking issues
The relay-email-input-wrapper
element that gets added around email input fields has the following CSS z-index value:
z-index: 99999999999999999;
I'm sure this property is likely needed for a reason, but its value is way too high. This results in the email field appearing visually stacked on top of pretty much any other element that may be on a website.
For example, sticky navigation:
@lesleyjanenorton I believe this is the underlying issue for a number of other website compat bug reports where the field is on top of other page elements?
completely true. Very annoying. z-order has an impact on UX.
Chiming in on this issue because it’s the earliest reference I can find to this particular bug, but it’s been mentioned elsewhere too. Setting a z-index
of 9999999999999999999999 (!??) is… much too much.
I see some PRs have tried to address this and have been closed without merging. I’d be happy to open a new PR pulling the various z-index
s back into the stratosphere. Is such a PR likely to get merged? I don’t want to waste anyone’s time if another solution is incoming.
Uninstalling Relay until this is fixed; it breaks a ton of websites and feels like bad web-citizenry.
I'm still experiencing the same issue on Firefox 95.0 and the latest version of Nightly (97.0, 2021-12-20) with this extension.
https://user-images.githubusercontent.com/17863082/146827395-4a04feda-b6bb-44c8-9342-20e133120e38.mp4
This is still present today. This addon breaks pages - come on, this is a major issue, why is it lying around for so long!
Also happens on https://accounts.nintendo.com. The email input is displayed on top of the CAPTCHA:
Does not occur if Relay extension is disabled (but 1password extension enabled)
I created a PR to fix this lingering issue: https://github.com/mozilla/fx-private-relay-add-on/pull/367 Would love to have someone from the Relay team review sometime soon!
Two. Years. This is so embarrassing.
I took a look at the 1passwrod extension to see how they handle this, and whilst they also use a very high z-index value, they add some clever additional behaviour to help minimise the issue:
- The icon is only displayed next to the input when the input is focused.
- The icon will hide / disappear again if the user scrolls the web page.
I think adding the two above items would likely help hide this issue in the majority of use-cases mentioned above.
Hi @alexgibson I would like to comment on this, which I assume affects many people using Relay, including me.
I've looked at how other extensions do this, and one extension in particular, Kee (Password Manager) (source here) does it in an interesting way so overlay issues don't really happen:
- Setting the background image of the input field to Relay's icon via style setters in JS
- Adjusting the size, position, etc to align at the right at an appropriate dimension.
- When the user hovers, it checks if it exceeds the threshold then based on that it shows the pointer icon, else the regular icon.
- Using the same check, display the overlay if user is detected clicking on the icon (based on threshold), else nothing is performed.
This should eliminate the need for the z-index
as well as the input overlay.
The relevant file is here: https://github.com/kee-org/browser-addon/blob/8ae4261c82578ea0f08f6ffe1409145872a81c96/page/keeFieldIcon.ts (in TypeScript), in the addIcon
function.
Based on their code, I have implemented a rudimentary version on Relay.
(Note: the code below has some errors, but nonetheless shows a PoC of the new method)
I changed the addRelayIconToInput
function in src/js/other-websites/add_input_icon.js
to this
async function addRelayIconToInput(emailInput) {
emailInput.style["background-image"] = "url('https://relay.firefox.com/_next/static/media/favicon.fc247257.svg')"
emailInput.style["background-repeat"] = "no-repeat"
emailInput.style["background-attachment"] = "scroll"
emailInput.style["background-size"] = "16px 16px"
emailInput.style["background-position"] = "calc(100% - 4px) 50%"
emailInput.style["cursor"] = "auto"
emailInput.addEventListener("mousemove", async (e) => {
if (!e.isTrusted || e.target.disabled) return
const rect = e.target.getBoundingClientRect()
const leftLimit = rect.left + rect.width - 22
// console.log(leftLimit)
// console.log(e.clientX)
if (e.clientX > leftLimit) {
e.target.style["cursor"] = "pointer"
// console.log("left")
return;
}
// console.log("not")
e.target.style["cursor"] = "auto"
})
//
const sendInPageEvent = (evtAction, evtLabel) => {
sendRelayEvent("In-page", evtAction, evtLabel);
};
emailInput.addEventListener("click", async (e) => {
if (!e.isTrusted) {
// The click was not user generated so ignore
return false;
}
const bcrect = e.target.getBoundingClientRect();
const leftLimit = bcrect.left + bcrect.width - 22;
if (e.clientX > leftLimit && bcrect.top <= e.clientY && e.clientY <= bcrect.bottom) {
lastClickedEmailInput = emailInput;
sendInPageEvent("input-icon-clicked", "input-icon");
preventDefaultBehavior(e);
window.addEventListener("resize", positionRelayMenu);
window.addEventListener("scroll", positionRelayMenu);
const signedInUser = await isUserSignedIn();
const relayInPageMenu = buildInpageIframe({isSignedIn: signedInUser});
const relayMenuWrapper = createElementWithClassList(
"div",
"fx-relay-menu-wrapper"
);
// Close menu if the user clicks outside of the menu
relayMenuWrapper.addEventListener("click", closeRelayInPageMenu);
// Close menu if it's already open
// relayIconBtn.classList.toggle("fx-relay-menu-open");
// if (!relayIconBtn.classList.contains("fx-relay-menu-open")) {
// return closeRelayInPageMenu();
// }
if (!signedInUser) {
addRelayMenuToPage(relayMenuWrapper, relayInPageMenu, emailInput);
sendInPageEvent("viewed-menu", "unauthenticated-user-input-menu");
return;
}
sendInPageEvent("viewed-menu", "authenticated-user-input-menu");
addRelayMenuToPage(relayMenuWrapper, relayInPageMenu, emailInput);
}
});
// divEl.appendChild(emailInput);
// emailInputWrapper.appendChild(divEl);
sendInPageEvent("input-icon-injected", "input-icon");
}
A screenshot of the proposed change:
Currently, the offsets for the overlay menu when clicked are not set properly but if I do a PR I'll fix them.
I can push a PR if you'd like/this change is acceptable, in which I will try to fix the issues with the current code and optimise it, delete unnecessary code, etc, and possibly re-implement it based on the same idea to avoid license issues.
Please do let me know if you'd like me to do so.
This may require a sizeable refactor to the code base though.
Thanks!
Dude, three years now. I am unsubscribing from this issue - it wont ever get fixed 😓
Thanks @czlucius, I actually have a prototype working with exactly that approach*, and it does appear to work well so far. Unfortunately it does require some more work to get that over the finish line, among which testing it on a list of commonly-used websites, which we have yet to find the time for, and, as you mentioned, a sizeable refactor of the codebase. So progress is being made, but it is slow, unfortunately.
* Well, roughly that approach - some extra work is needed to ensure that people with visual or motor impairments can use it, but I've got solutions for that too.
Oh I see. That's good (I mean for the progress you've made to get it to the stage of a working prototype). May I help out in that?
Not much I think, unfortunately. The main thing is that the team needs to review and get familiar with the refactored codebase so they can build on top of it, so we need time allocated there - which of course is competing with all the other things people are clamouring for. Sorry about that - personally, I'm really hoping we can get this done soon, but of course, it's not just up to me.
Ok I hope the team(at Relay) would be able to get this ready soon. Thanks