martok / palefill

Inject Polyfills for various web technologies into pages requiring them
https://martok.github.io/palefill/
Mozilla Public License 2.0
79 stars 9 forks source link

CustomElements Event propagation broken #30

Closed martok closed 1 year ago

martok commented 2 years ago

Among other things, soft navigation in github doesn't work. This is because the event retargeting shim is extremely limited and not spec compliant.

A proper implementation could look something like https://github.com/salesforce/lwc/blob/master/packages/%40lwc/synthetic-shadow/src/faux-shadow/events.ts

UCyborg commented 2 years ago

I wonder if this is also the reason reacting to a comment (only tried the one on commit page) doesn't show until you refresh the page.

martok commented 2 years ago

Event propagation is weird. Feels broken.

The reason why SeaHOH implemented this "button.onclick" special case is that the event doesn't get dispatched to the custom element inside the button. Almost as if they are not considered valid event targets? I have no idea why this is the case. Normally, the only gotcha in the WebComponents world is that emulated shadow roots don't receive events, their host does and so one has to manually forward the event to where the listeners are attached. But that is an entirely different story to basic custom elements that should just behave like real elements.

In the end I rewrote a lot and ended up just taking the same solution to get it working...

I wonder if this is also the reason reacting to a comment (only tried the one on commit page) doesn't show until you refresh the page.

Could be, it works for me with the new code.

martok commented 2 years ago

This one special case is fixed, but in general custom elements have some sort deeper issue where they sometimes steal events from their children and sometimes their children bubble events to the wrong level.

martok commented 1 year ago

The reason why SeaHOH implemented this "button.onclick" special case is that the event doesn't get dispatched to the custom element inside the button. Almost as if they are not considered valid event targets?

This is indeed exactly the problem with the "Copy SHA" button. It is not the <button> that has the event attached but the <clipboard-copy> CE. The CE is then made big enough to cover the entire button, so it "looks like" the user presses a button. Easy test: there are a few pixels on the left where the button is larger than the CE where Chromium doesn't copy anything, same as Pale Moon. I've set a reddish background for the CE here so it's visible: image

With keyboard navigation, same thing: the button doesn't do anything, the CE must be focused. Which interestingly works in Pale Moon! Now, the question is: why does the customElement not receive mouse events. It doesn' even capture the DevTools inspector context menu.

SeaHOH commented 1 year ago

This must be a bug of <button>, other custom elements can receive mouse events.

martok commented 1 year ago

Wouldn't be the first, the thing that confuses me is why it's only mouse events and keyboard works.

Although... the hint bug that @Vangelis66 found on the other issue could be a hit test issue as well. Assuming an element inside <button> doesn't take part in hit tests, it's hint wouldn't be shown as well as clicks not delivered.

Likely MozCentral 1089326 (the "4 years ago" part), diff Maybe also related 1598684 diff

SeaHOH commented 1 year ago

Polyfill is weak, there is nothing more we can do, just have to wait Moonchild to fix it.

martok commented 1 year ago

... or contribute to Pale Moon. Which I intend to do with these (assuming I can still build it), the links were mostly notes to myself.

Other than that, the polyfill I've been working on over the weekend should fix the remaining event issues. Just need to async-ify it a bit and be a bit less paranoid about DOM updates, currently it is way too slow.

martok commented 1 year ago

currently it is way too slow.

Show of hands: is ~1.5s slower (using this issue page as test case) compared to Palefill 1.23 acceptable?

For me, that changes DOMContentLoaded (using Network panel of devtools) from 10s to 11.5s, which is overall barely noticeable difference. Of that, the first 5s are HTML parsing, the first JS task gets processed at around 6s and the last shadow operation is at about 9s.

garoto commented 1 year ago

Is this issue related to the timestamping one and to your PR in palemoon's repo that Vangelis66 mentioned some hours ago? Because I have no idea what soft navigation is, but otherwise, as long the 1.5s increase in rendering time is the penalty for having a non-broken GH, then I raise my hands I guess.

Vangelis66 commented 1 year ago

Thanks @martok for your efforts, all the more since you're pressed for spare time...

If "soft-navigation" is to come back, will (currently open) #53 be addressed? And, are you talking about this? My primary concern is that #60 is somehow addressed, because the lack of relative timestamps slows down my workflow 😞 ...

As for longer page-rendering times, I have mixed feelings... Obviously, the times measured also depend on the underlying hardware (CPU+GPU+RAM), not only on the code running; (I guess) people on older machines (and older OSes using the UXP forks) will be affected more than, say, a user on an i7+16GB of RAM on Win10x64...

Despite the above reservation, you have my blessing to go forward with whatever is deemed necessary to get GitHub fully working again (until MS, on a whim, break it again for us 😠 ) ...

@garoto

I have no idea what soft navigation is

From "https://github.com/WICG/soft-navigations#readme":

"Soft navigations" are JS-driven same-document navigations that are using the history API or the new Navigation API, triggered by a user gesture and modify the DOM, modifying the previous content, as well as the URL displayed to the user.

and from "https://www.infoq.com/news/2015/11/performance-web-applications/":

Soft navigations are not real navigations, that is, you can change a browser's URL without really navigating and thus, without triggering onload events. And the browsers don't have an API that informs if and when all the resources - i.e.: images, scripts, CSS, videos - have been downloaded.

TL;DR: Say you visit https://github.com/martok/palefill in Google Chrome/Mozilla Firefox; by default, you should land in the <>Code tab; when you click, e.g., the Insights tab (within the same GH page), this is loaded via soft-navigation; the browser's URL bar will change to "https://github.com/martok/palefill/pulse", while in the process of doing so GitHub will briefly display a thin blue progress line (from left to right) at the top of the page...

Is this issue related to ... your PR in palemoon's repo that Vangelis66 mentioned some hours ago?

The UXP PR I mentioned in the other issue intends to remedy the bug discussed here and here ...

garoto commented 1 year ago

Alright, cheers Vangelis66 for the detailed explanation.

martok commented 1 year ago

Thanks @martok for your efforts, all the more since you're pressed for spare time...

It's getting better (or my time management is), and today I've got notification that a grant proposal deadline I expected for April is going to be in June :smile:

If "soft-navigation" is to come back, will (currently open) #53 be addressed?

I'm not sure. Something there causes massive memory leaks (see #50), so it doesn't seem worth it. We'll see.

And, are you talking about this?

Yep. Or rather, a much more refined version of that concept (turns out it can't work like that) that will also be released as a separate project. Because who doesn't want yet another ShadowDOM polyfill in their life :monocle_face:

My primary concern is that #60 is somehow addressed, because the lack of relative timestamps slows down my workflow disappointed ...

That, and hopefully a few others. Haven't had a release in a while and stuff piles up... should be out before the end of the week, I also want to retest a few old bugs and see if we can get away with less weird patches now.

As for longer page-rendering times, I have mixed feelings...

Same, but honestly: if you want a fast experience, don't use Pale Moon. I have no idea why that is, but complex documents even without any scripting are so much slower than in other browsers, it's almost funny given how I discovered PM waaaay back when it was "just" a more optimized build of Firefox.

The UXP PR I mentioned in the other issue intends to remedy the bug discussed here and here ...

That just got merged an hour ago :)

martok commented 1 year ago

Well, we know where the remaining issue comes from and I've likely fixed everything regarding element propagation that can be fixed. So I'll close this and leave issue #64 to track that one problem.