Summary: | [Wheel event region] Add basic support for generating accurate wheel event listener region | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bfulgham, cdumez, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, kangil.han, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, zalan | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=211536 | ||||||||||||||
Attachments: |
|
Description
Antti Koivisto
2020-05-06 07:24:53 PDT
Created attachment 398611 [details]
patch
Created attachment 398614 [details]
patch
Comment on attachment 398614 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=398614&action=review > Source/WebCore/rendering/style/StyleRareInheritedData.h:158 > + unsigned eventListenerRegionTypes : 2; // OptionSet<EventListenerRegionType> It's going to be really easy to forgot to change this when we add more EventListenerRegionType bits. I think maybe just eat the 8 bits right now, or add a compile-time assert that things will fit. > Source/WebCore/style/StyleAdjuster.cpp:219 > + auto findListeners = [&](auto eventName, auto type, auto nonPassiveType) { > + auto* eventListenerVector = element.eventTargetData()->eventListenerMap.find(eventName); > + if (!eventListenerVector) > + return; > + > + types.add(type); > + > + bool passiveOnly = true; > + for (auto& listener : *eventListenerVector) { > + if (!listener->isPassive()) > + passiveOnly = false; > + } > + > + if (!passiveOnly) > + types.add(nonPassiveType); > + }; Maybe this should be a function on Element Comment on attachment 398614 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=398614&action=review >> Source/WebCore/rendering/style/StyleRareInheritedData.h:158 >> + unsigned eventListenerRegionTypes : 2; // OptionSet<EventListenerRegionType> > > It's going to be really easy to forgot to change this when we add more EventListenerRegionType bits. I think maybe just eat the 8 bits right now, or add a compile-time assert that things will fit. Patch is ok as is. No change needed. Just for me: Could this storage also be used to track the editable region? Usually only elements with userModify != ReadOnly are in that region, but there's one except for HTMLTextFormControl elements that is currently handled by plumbing an overrideUserModifyIsEditable book through EventRegionContext::unite(). This storage could be used to eliminate that bool. I didn't want to add storage to every element's style before, but now that this storage exists it could be used. Just a thought.... Created attachment 398625 [details]
patch
> Maybe this should be a function on Element
Nah, element is the same for all callers.
Created attachment 398629 [details]
patch
> Just for me: Could this storage also be used to track the editable region?
Probably.
Committed r261239: <https://trac.webkit.org/changeset/261239> All reviewed patches have been landed. Closing bug and clearing flags on attachment 398629 [details]. As indicated by mac-debug-wk1 EWS, this change caused fast/events/wheel-event-outside-body.html to assert SHOULD NEVER BE REACHED ./rendering/RenderLayerBacking.cpp(2948) : void WebCore::RenderLayerBacking::paintIntoLayer(const WebCore::GraphicsLayer *, WebCore::GraphicsContext &, const WebCore::IntRect &, OptionSet<WebCore::PaintBehavior>, WebCore::EventRegionContext *) https://bugs.webkit.org/show_bug.cgi?id=211536 Reverted r261239 for reason: Caused fast/events/wheel-event-outside-body.html to assert on macOS WK1 Committed r261253: <https://trac.webkit.org/changeset/261253> Created attachment 398717 [details]
patch
Committed r261279: <https://trac.webkit.org/changeset/261279> All reviewed patches have been landed. Closing bug and clearing flags on attachment 398717 [details]. |