Summary: | Add support for a UI delegate method to decide how to handle detected modal containers | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||||||
Component: | Platform | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | akeerthi, annulen, bdakin, cdumez, darin, dino, ews-watchlist, graouts, gyuyoung.kim, hi, japhet, megan_gardner, ryuan.choi, sergio, thorton, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 234322, 234323 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Wenson Hsieh
2021-12-17 13:08:42 PST
Created attachment 447472 [details]
Patch
Created attachment 447667 [details]
Depends on #234322
Created attachment 447678 [details]
Fix Windows build
Comment on attachment 447678 [details] Fix Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=447678&action=review > Source/WebCore/loader/DocumentLoader.h:151 > -enum class ModalContainerObservationPolicy : uint8_t { > +enum class ModalContainerObservationPolicy : bool { > Disabled, > Prompt, > - Allow, > - Disallow, > }; I really prefer that such simple enums are done as one liners instead of vertically. Possibly others don’t agree. > Source/WebCore/page/ModalContainerObserver.cpp:314 > + ClassifiedControls(ClassifiedControls&& other) > + : positive(WTFMove(other.positive)) > + , neutral(WTFMove(other.neutral)) > + , negative(WTFMove(other.negative)) > + { > + } This can just be: ClassifiedControls(ClassifiedControls&&) = default; But also, if you just omit all constructors you should get these without writing anything. > Source/WebCore/page/ModalContainerObserver.cpp:326 > + case ModalContainerDecision::Show: > + break; > + case ModalContainerDecision::HideAndIgnore: > + break; Idiomatic to just share a break. > Source/WebCore/page/ModalContainerObserver.cpp:341 > + OptionSet<ModalContainerControlType> controlTypes() const In the context of a class named ClassifiedControls, seems this can just be called types(). Comment on attachment 447678 [details] Fix Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=447678&action=review Thanks for the review! >> Source/WebCore/loader/DocumentLoader.h:151 >> }; > > I really prefer that such simple enums are done as one liners instead of vertically. Possibly others don’t agree. Makes sense! Changed to `enum class ModalContainerObservationPolicy : bool { Disabled, Prompt };`. >> Source/WebCore/page/ModalContainerObserver.cpp:314 >> + } > > This can just be: > > ClassifiedControls(ClassifiedControls&&) = default; > > But also, if you just omit all constructors you should get these without writing anything. Good catch — removed these constructors. >> Source/WebCore/page/ModalContainerObserver.cpp:326 >> + break; > > Idiomatic to just share a break. Fixed! >> Source/WebCore/page/ModalContainerObserver.cpp:341 >> + OptionSet<ModalContainerControlType> controlTypes() const > > In the context of a class named ClassifiedControls, seems this can just be called types(). That's true — renamed to just `types()`. Created attachment 447719 [details]
For EWS
Created attachment 447721 [details]
Fix Windows build
Committed r287321 (245471@main): <https://commits.webkit.org/245471@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447721 [details]. |