Widget Replacement button too high on z-axis

[colinje]

Description

The widget replacement for Facebook has a button that appears to be hoisted into the heavens on the z-axis.
This may be a result of how Downdetector's header is styled, but if the button was just laid on top of the widget, I wouldn't think this would happen.

Steps to Reproduce

1. Navigate to https://downdetector.com
2. Scroll down until the position of the "Unblock" button of replacement widget is in the same space as the top nav

Expected behavior:
Button tucks under the heading

Actual behavior:

Versions

• Extension: 2022.3.30
• Browser: Firefox 99
• OS: Windows 10 Home

[jonathanKingston]

Hey! Thanks for reporting this!

/cc @kzar

[mighty-phoenix]

Hey! @jonathanKingston @kzar @sammacbeth
Can I take up this issue for starters?
My name is Rakshit.
I would love to start contributing to the DuckDuckGo privacy extension. DuckDuckGo's mission for privacy inspires me.
I plan to work with DuckDuckGo repositories regularly and wish to get involved with the development teams to pick more issues to solve.
Can you please help me with that?
Thanks!

[jonathanKingston]

Hey @mighty-phoenix thanks for the PR. I think this might be the correct fix as the injected DOM elements sits inside with the original page and so most of the time should inherit the z-index from it's parent element.

There are 2 other cases in this code that also inject high z-index values and I suspect they should get the same fix; whatever that is.

I think when @Charlie-belmer first made this code there were some cases where we were sitting behind the pages elements and perhaps he can remember as test cases. If that is the case I think we may want to use JS to actually inspect the page z-index and use the correct values based on the surrounding nodes; this however might be equally fraught with the same issues.

<header style="position: fixed; z-index: 12"></header>
<section style="z-index: 11">
    <button>Facebook button</button>
</section>

Picking the correct value for the above element is difficult as using the same z-index will be incorrect on some sites but also +1 is also incorrect in the case above.

I'm inclined to suggest this is the correct fix but will defer to @Charlie-belmer, @kzar or @ladamski but as mentioned I think the other z-index items should be removed too if it's deemed correct.

[colinje]

I definitely don't know how the DDG widget is getting displayed, but the "Unblock Content" button is a child of your widget, right? Why would you hoist the button? If a website is setting z-index and the button happens to occupy the same or a tad higher z-index, then that's not DDG's problem. That's just a poorly made website. The best you could do is as you suggest, setting the z-index of what you're injecting, to the same z-index of what you are replacing AND the widget would have to be completely flat. (no parents, all sisters, to guarantee that everything is displayed at that "layer". instead of injecting a parent element with children, inject a list of elements all at the z-index layer. this implementation is not pretty, but it would help ensure that at least DDG's widget would look pretty)

I'd have to really dig in to understand how the widget is getting injected to help propose a solution.

And I would also be interested in the test cases that drove the decision to hoist the button in the first place.

[jonathanKingston]

The button isn't always embedded within the section as shown on this site. You can see some other examples here: https://privacy-test-pages.glitch.me/privacy-protections/click-to-load/ I think the single button case is what we saw breakage with rather than the whole panel element; we could choose to remove the z-index when embedded within the panel as we're not applying a z-index to the panel and the panel will already broken in these edge cases.

That's just a poorly made website. The best you could do is as you suggest, setting the z-index of what you're injecting, to the same z-index of what you are replacing.

If I understand correctly there are sites that are setting the z-index on the DOM nodes created by Facebook; which may not be able to understand we have modified the output. We are replacing the nodes before they are created so can't establish what the z-index might be.

I think we need some clear test cases that were breaking and were the rationale for the original decision to make a decision on the next steps here.

DuckDuckGoPrivacyEssentialsHoverableText we probably always want to be positioned above the rest of the DOM which probably would be better served by showModal().

[Charlie-belmer]

I went back through the original test cases (real, popular global sites) - unfortunately many of those have changed or removed FB content, so I couldn't find the original sites causing this issue.

If I recall, the root of the problem was sites with global button styles that would set z-index below the facebook parent element, effectively hiding the button and making it impossible to click. At the time, I chose to ensure clickability over cosmetics, but it's entirely possible the current z-index causes more problems than it solves. Either way, I think we risk some breakage.

I think it's up to @kzar and @ladamski to make the call since they own this feature now.

[colinje]

Is making it impossible to unblock a tracker a bad thing? (ha) I originally thought this would be one of those simple requests that would be a good first issue. (Thank you @mighty-phoenix for finding this and dedicating time to investigate, as well as @jonathanKingston and @Charlie-belmer for spending valuable time researching the origins of this.)

"global button styles that would set z-index below the facebook parent element"....acknowledged, and I can appreciate function over fashion. Would setting "z-index: revert" undo that global style if it existed? (I've honestly never used "revert" before, but if I understand correctly, this sounds really useful for this use case, if not other "click to play" features?)

[Charlie-belmer]

I'm not sure what z-index: revert does, I've never used it myself and am not familiar with it. On the other hand, there may be other creative ways to solve this using stacking contexts. It would be helpful to first create at test case with some global styles that replicate the original & current issues (maybe in https://github.com/duckduckgo/privacy-test-pages/tree/main/privacy-protections/click-to-load)?

Then we could be more confident that whatever fix we implement solves both issues.