[webkit-reviews] review granted: [Bug 224549] Move FocusRemovalEventsMode into FocusOptions : [Attachment 426041] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Manuel Rego Casasnovas
<rego at igalia.com>'s request for review:
Bug 224549: Move FocusRemovalEventsMode into FocusOptions
https://bugs.webkit.org/show_bug.cgi?id=224549

Attachment 426041: Patch

https://bugs.webkit.org/attachment.cgi?id=426041&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 426041
  --> https://bugs.webkit.org/attachment.cgi?id=426041
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=426041&action=review

>> Source/WebCore/dom/Document.cpp:4363
>> +	    setFocusedElement(nullptr, focusOptions);
> 
> I've initially used the following line here, but it doesn't build on Windows
port due to the C++ version:
>	setFocusedElement(nullptr, { .removalEventsMode =
FocusRemovalEventsMode::DoNotDispatch });

Yes, I think that might be a C++20 feature and we can’t start using those yet.
But soon I hope!

But maybe you could write something like this:

    setFocusedElement(nullptr, { { }, { }, {
FocusRemovalEventsMode::DoNotDispatch }, { } });

While it’s not super-beautiful, there’s no chance of the FocusRemovalEventsMode
ending up in the wrong structure element, nor of anything else being set to a
non-default value.

> Source/WebCore/dom/FocusRemovalEventsMode.h:30
> +enum class FocusRemovalEventsMode { Dispatch, DoNotDispatch };

Do we really need a separate header for this? Can we just put this into
FocusOptions.h and include that everywhere this is needed?

I’m willing to have a separate header for a single enumeration if it’s really
helpful but it would be good to avoid if possible.