Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Events whose composed flag should be true #513

Closed
hayatoito opened this issue Jun 3, 2016 · 56 comments
Closed

Events whose composed flag should be true #513

hayatoito opened this issue Jun 3, 2016 · 56 comments

Comments

@hayatoito
Copy link
Contributor

hayatoito commented Jun 3, 2016

We have decided to use Event.composed flag (false by default), instead of Event.scoped, negating its meaning. See the context: whatwg/html#1160.

That means we should have a list of exceptional events whose composed flag should be true,
instead of the current list.

Eventually, instead of having a list in one place, each event's definition should mention its composed flag should be true in each place, but I would like to have a rough consensus here.

I have made a list of events whose composed flag should be true by default, I think. The list is basically made from UIEvent spec: https://w3c.github.io/uievents/.

  • Focus Events: blur, focus, focusin, focusout
  • Mouse Events: click, dblclick, mousedown, mouseenter, mousemove, mosueout, mouseover, mouseup
  • Wheel Events: wheel
  • Input Events: beforeinput, input
  • Keyboard Events: keydown, keyup
  • Composition Events: compositionstart, compositionupdate, compositionend

I am going to update Blink's implementation, however, what do you think about Composition Events? I am not 100% sure what should I do for Composiition Events. Should we exclude Composition Events from here?

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 3, 2016

@sorvell, I would like to hear opinions from Polymer folks too. Does the decision affect Polymer's implementation?

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 3, 2016

TouchEvents and PointerEvents are also candidates.

Given that, instead of enumerating every candidates,
can we just say: "Every UIEvents, except for load, unload, abort, error and select"?

Is that okay?

@annevk
Copy link
Collaborator

annevk commented Jun 6, 2016

We really have to update the relevant specifications that dispatch UI events. And if you really want to create a monkey patch first, I think it's better to be exhaustive than to hope implementations will do the right thing.

@rniwa
Copy link
Collaborator

rniwa commented Jun 6, 2016

We should explicitly enumerate every event.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 6, 2016

I agree that we should explicitly enumerate every event. Let me try to enumerate every event here before updating the relevant specifications.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 7, 2016

Here is the exhaustive list. This should be a good starting point, hopefully.

I think it is important that we can know the rationale how we can make the list.
So here is how to reproduce the list.

1. Go to https://w3c.github.io/uievents/.
   Get every event types from Chapter 5, excluding events in Chapter 5.1
   -> We got:
      Focus Events:
        blur
        focus
        focusin
        focusout
      Mouse Events:
        click
        dblclick
        mousedown
        mouseenter
        mouseleave
        mousemove
        mouseout
        mouseover
        mouseup
      Wheel Events:
        wheel
      Input Events:
        beforeinput
        input
      Keyboard Events:
        keydown
        keyup
      Composition Events:
        compositionstart
        compositionupdate
        compositionend
2. Go to https://w3c.github.io/touch-events/
   -> We got:
      Touch Event:
        touchstart
        touchend
        touchmove
        touchcancel
3. Go to https://w3c.github.io/pointerevents/
   -> We got:
      Pointer Events:
        pointerover
        pointerenter
        pointerdown
        pointermove
        pointerup
        pointercancel
        pointerout
        pointerleave
        gotpointercapture
        lostpointercapture

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 7, 2016

Yeah, I think per interface basis might work well here because I could not find any exceptional case.

However, I do not agree the rationale you mentioned :)
For synthetic events, we should always honor the EventInit dictionary.
I meant:

let e = new FocusEvent('focus');
console.assert(e.composed == false);

let e = new FocusEvent('focus', { composed: false });
console.assert(e.composed == false);

let e = new FocusEvent('focus', { composed: true });
console.assert(e.composed == true);  <== true only if it is specified in EventInit dictionaly

@domenic
Copy link
Collaborator

domenic commented Jun 7, 2016

It's worth checking to see if https://html.spec.whatwg.org/multipage/indices.html#events-2 has any events you'll want to include. My guess is that most of them that could be considered "UI events" are already in those other specs but some might be missing. E.g. I think change or copy or cut or paste might be worth considering.

@annevk
Copy link
Collaborator

annevk commented Jun 7, 2016

Drag-and-drop too probably. Drag-and-drop and cut/copy could be tricky with the user leaking nodes from shadow trees. Maybe that's okay...

@rniwa
Copy link
Collaborator

rniwa commented Jun 7, 2016

However, I do not agree the rationale you mentioned :)
For synthetic events, we should always honor the EventInit dictionary.

I agree in the case author specified a value but we should just change the default value for EventInit for those special constructors when the value is missing.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 8, 2016

I agree in the case author specified a value but we should just change the default value for EventInit for those special constructors when the value is missing.

Can we do that? I am afraid that it is confusing if EventInit's default value depends on Event's interface.

dictionary EventInit {
  boolean bubbles = false;
  boolean cancelable = false;
  boolean composed = false;
};

I prefer not changing the default value, per Event interface.
Rather, I would like to think as if UA set it to true explicitly, like the following:

// This is *imaginary* code, which is happening in browser's engine for UA events.
let e = new FocusEvent('xxx', { composed: true });

Thus, I would like users to specify it explicitly as UA does.

@hayatoito
Copy link
Contributor Author

It looks that DragEvent inherits MouseEvent. Thus, this can be automatic if we use per-interface basis.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 8, 2016

cut/copy/paste are defined in https://w3c.github.io/clipboard-apis/

Its interface is

interface ClipboardEvent : Event

I have taken a look at the spec, however, I do not have any preference whether to add ClipBoardEvent or not. I guess there are different opinions on ClipboardEvent.

It might be good to exclude it as a starting point. If web developers find that it is inconvenient, we can add it without a significant risk, I think.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 8, 2016

In summary, so far:

Inheritance hierarchy:

  • UIEvent
    • FocusEvent
    • MouseEvent
      • WheelEvent
      • PointerEvent
      • DragEvent
    • InputEvent
    • KeyboardEvent
    • CompositionEvent
    • TouchEvent

If we use per-interface basss, FocusEvent, MouseEvent, InputEvent, KeyboardEvent, CompositionEvent, and TouchEvent are the candidates.

@rniwa
Copy link
Collaborator

rniwa commented Jun 8, 2016

I prefer not changing the default value, per Event interface.
Rather, I would like to think as if UA set it to true explicitly, like the following:

// This is *imaginary* code, which is happening in browser's engine for UA events.
let e = new FocusEvent('xxx', { composed: true });

Thus, I would like users to specify it explicitly as UA does.

The thing is, FocusEvent doesn't take EventInit. It takes FocusEventInit, which inherits from UIEventInit, which in turn inherits from EventInit. In the case of MouseEvent and its derived classes, MouseEventInit doesn't even inherit from EventInit (this is presumably a spec bug though). In general, there should be a minimal difference between UA created events and author created events.

If we're not going to the route of initializing based on the interface, then we absolutely need to enumerate every single event for which composed flag must be set to true by UA instead of vaguely relying on the interface being used.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 8, 2016

If possible, I would like to adopt per-interface basis for synthetic events too, but my conern is its feasibility.

Suppose the following cases:

 let e1 = new FocusEvent('xxx');
 let e2 = new FocusEvent('xxx', { bubbles: true });
 let e3 = new FocusEvent('xxx', { bubbles: true , composed: true });
 let e4 = new FocusEvent('xxx', { bubbles: true , composed: false});

If we adopt "default changes per-interface" for synthetic events too, I think user's expectations are:

 assert(e1.composed === true);
 assert(e2.composed === true);
 assert(e3.composed === true);
 assert(e4.composed === false);

However, since FocusEventInit inherits EventInit, we can not distinguish case e2 and e4 nicely in our binding layer. In both cases, FocusEventInit.composed was already set to false. We can not tell the difference.

@rniwa
Copy link
Collaborator

rniwa commented Jun 8, 2016

I don't understand how that works. You're invoking a different constructor. Surely you ought be able to distinguish that FocusEvent rather than Event constructor is invoked. Then, in each constructor, you can specify the appropriate default value for composed when it's missing in the dictionary specified by the author.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 8, 2016

I have found that Blink's Web IDL binding generates the following for EventInit:

https://cs.chromium.org/chromium/src/out/Debug/gen/blink/core/events/EventInit.h

Nullable<bool> m_composed;

It is not bool. It is Nullable<bool>! I did not realize it. It looks that we can distinguish e2 (missing (m_composed == null , hopefully) and e4 (m_composed == false).

However, does WebIDL guarantee this third state? Do every user agents can tell whether the value is missing, false or true, strictly?

@domenic
Copy link
Collaborator

domenic commented Jun 8, 2016

I don't have a very strong opinion here but I wanted to say that the direction this discussion is going seems rather strange to me. My mental model is that the composedness of an event is similar to whether it bubbles or is cancelable. That is, it is a property of an individual firing of the event. Saying that there should be a per-interface default for composedness seems very strange to me given that there is no similar concept for bubbles or cancelable.

As far as I can tell every input event fired by the platform bubbles. But we don't say that InputEvent should have a default of bubbles: true. We don't say that new InputEvent("input", {}) should be equivalent to new InputEvent("input", { bubbles: true }). Instead we explicitly specify the value of the bubbles flag at all places in the platform that fire input events.

I can see some value in providing non-normative guidance that in general events which use the InputEvent interface when fired by the platform will bubble, not be cancelable, and be composed. Maybe there could be a table listing this matrix for all event types and/or interfaces. But I don't think that should be part of the definition of InputEvent or InputEventInit, and should not affect the developer-facing defaults, which IMO should be the same for all events.

Yes, it will be a lot of work to update all the places that fire events to correctly set composed. But it is doable, and would be consistent with how the platform already works for other flags on events. I don't think we should try to take a shortcut here.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 8, 2016

Yeah, regarding UA events, I think "per-interface basis" is just a syntax sugar which is only valid in this discussion.
In updating the relevant specs, we should update each place where an event is fired.

Enumerating interfaces here should be just a guide which tells us where we should update.

@rniwa
Copy link
Collaborator

rniwa commented Jun 8, 2016

@domenic : That's a fair point. Let's go with explicitly enumerating every single that UA fires as composed=true then.

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 8, 2016

Thank you, folks. I think the conclusion is:

  1. For each relevant spec:
    1. For each place where an event is fired:
      1. If the event's interface is (or inherits) one of the enumerated interfaces here:
        1. Set composed=true

In terms of implementation, I think this conclusion is implementor-friendly because we do not have to update many places in our engine.

@hayatoito
Copy link
Contributor Author

I have filed an issue for UIEvents specification: w3c/uievents#89

@hayatoito
Copy link
Contributor Author

I think we have to reflect our conclusions to the following specifications. I have filed issues:

hayatoito added a commit to hayatoito/pointerevents that referenced this issue Jun 21, 2016
All pointer events should propagate across Shadow DOM bounaries.
See WICG/webcomponents#513 for details.
@hayatoito
Copy link
Contributor Author

hayatoito commented Aug 9, 2016

Thank you for the feedback.
In such a user interaction, a composed event is always fired, isn't it? e.g. "mouseover" or "click" event.
Is it difficult for component consumers to do the right thing at the right timing only by listening these composed events?

@ebidel
Copy link

ebidel commented Aug 9, 2016

Sure, you could listen for something like "click", but I'm not sure how you'd accurately know when the animation/transition is done. Plus, the last thing you'd want is user code (potentially) running during your animation.

A couple of more useful examples from the Google I/O 2016 PWA (we waited for several components to fire a transitionend event for internal animations, to delay work and make sure animations were smooth).

  • a photo gallery - manages a11y behavior when a new photo is transitioned to.
  • <countdown-timer> that fades itself out when the time is up. User code hooks into the transition and sets state when it's finally invisible.

@domenic
Copy link
Collaborator

domenic commented Aug 9, 2016

It seems really bad to be reusing animation events for those kind of domain specific "done sliding" things. I would expect you would fire control-specific events like "toggle" or "open" or "close".

Otherwise you are tying your consumers to the implementation detail of your component, which is that it currently animates. What happens when in v2 you implement a "low power mode" that stops doing animations when the battery level is low? Or in v13 when the industry has moved away from animations as a method of signifying transitions (like Apple has largely done from OS X 10.0 to the current macOS versions)?

Transition/animation events definitely feel like the kind of internal-to-a-control, implementation-choice-specific, presentation-related events that should not go outside the shadow tree by default.

@hayatoito
Copy link
Contributor Author

hayatoito commented Aug 9, 2016

@ebidel Thanks.

Could components authors catch these animation events in their shadow tree, and fire a component specific custom event for component consumers?

As the example does it, "on-countdown-complete", in https://github.com/GoogleChrome/ioweb2016/blob/41bdeb6ad3a41fe675f2b49dfa343121f720c135/app/elements/io-live.html#L270

Does it make an interface between component authors and component users more stable one?

@ebidel
Copy link

ebidel commented Aug 9, 2016

This was my earlier point:

The component could fire a custom event, but that requires more boilerplate for its author (has to add listeners, catch it, forward a different name, and do cleanup work in detached). It's more natural to use the animation events already being fired.

IMO, that's all more complicated than it needs to be.

What happens when in v2...

Events are part of a component's public API. I'd expect well authored components to clearly document changes. Real example: https://elements.polymer-project.org/elements/paper-button#events

@domenic
Copy link
Collaborator

domenic commented Aug 9, 2016

Right. I think the concern is exactly about making it easy to accidentally expose implementation details as part of a component's public API. Doing so should be intentional.

@hayatoito
Copy link
Contributor Author

hayatoito commented Aug 9, 2016

Thank you for the feedback.

For compatibility issues, a naive way of not breaking public API might be "catch an animation event in the shadow tree, and fire a new synthetic (the same kind of) animation event for the host". I am aware that this is all more complicated that it needs to be. :(

Could it be an acceptable work around???

I am not sure what is the best solution to address this issue. I would like to hear opinions from other browser vendors.

@rniwa
Copy link
Collaborator

rniwa commented Aug 9, 2016

This was my earlier point:

The component could fire a custom event, but that requires more boilerplate for its author (has to add listeners, catch it, forward a different name, and do cleanup work in detached). It's more natural to use the animation events already being fired.

IMO, that's all more complicated than it needs to be.

I could imagine adding some sort of a hook in v2 to make this easier. That is, add some API to specify whether a given event should propagate out of the current shadow tree.

But in general, the same argument could be used to make almost every event composed, and we don't want to do that at least for v1.

@annevk
Copy link
Collaborator

annevk commented Aug 9, 2016

I agree with @rniwa. A v2 API where you can override some of this on the shadow root sounds interesting and might be worthwhile.

@jsilvermist
Copy link

jsilvermist commented Mar 23, 2017

The fullscreenchange event (and fullscreenerror) which is a part of the Fullscreen API and is currently implemented in most browsers behind a prefix is not being fired with composed, making the Fullscreen API much harder to use.

@annevk
Copy link
Collaborator

annevk commented Mar 24, 2017

@jsilvermist you're looking for #614.

@jsilvermist
Copy link

@annevk Yes... Yes I was... Thanks!

matteoceraico pushed a commit to matteoceraico/webkit that referenced this issue Jul 5, 2017
https://bugs.webkit.org/show_bug.cgi?id=158415

Reviewed by Chris Dumez.

Source/WebCore:

Replace `scoped` flag with `composed` flag and negate its meaning per the latest spec:
https://dom.spec.whatwg.org/#dom-event-composed
WICG/webcomponents#513

In the old spec, every event was assumed to be "composed" (crosses shadow boundaries)
by default and there was `scoped` flag which prevented the event from crossing bondaries,
and there was a handful of events for which `scoped` was set true when dispatched by UA.

In the new spec, every event is assumed to be "scoped" and a handful of user-initiated
events set `composed` flag to true, which is also exposed in EventInit dictionary.
`relatedTargetScoped` flag has been removed. New behavior is identical to when this flag
was set to true.

No new tests since existing tests are updated to test the new flag and behavior.

* dom/CompositionEvent.cpp:
(WebCore::CompositionEvent::isCompositionEvent): Added.
* dom/CompositionEvent.h:
* dom/Event.cpp:
(WebCore::Event::Event): Initialize m_composed. Also re-ordered m_type and m_isInitialized
for better packing.
(WebCore::Event::composed): Renamed from Event::composed. We return true whenever composed
is set to true in EventInit, or the engine is dispatching an user-initiated event listed in:
WICG/webcomponents#513 (comment)
as well as keypress, cut, paste, and, copy as discussed in:
WICG/webcomponents#513 (comment)
(WebCore::Event::isCompositionEvent): Added.
* dom/Event.h:
(WebCore::Event::composed): Added.
(WebCore::Event::scoped): Deleted.
(WebCore::Event::relatedTargetScoped): Deleted.
(WebCore::Event): Reordered m_type and m_isInitialized for better packing. Added m_composed
and removed m_scoped and m_relatedTargetScoped.
* dom/Event.idl:
* dom/EventPath.cpp:
(WebCore::shouldEventCrossShadowBoundary): Returns true if the event did not originate from
a shadow tree (this event entered the current shadow tree via a slot so we need to proceed with
the normal bubble path outside the shadow tree) or composed flag is set true.
(WebCore::EventPath::EventPath): m_event no longer exists, which was only used to get the value
of relatedTargetScoped which has been removed.
(WebCore::EventPath::setRelatedTarget): Behave as if relatedTargetScoped is always set true
since the flag has been removed.
* dom/EventPath.h:
* dom/FocusEvent.cpp:
(WebCore::FocusEvent::relatedTargetScoped): Deleted.
* dom/FocusEvent.h:
* dom/MouseEvent.cpp:
(WebCore::MouseEvent::relatedTargetScoped): Deleted.
* dom/MouseEvent.h:

LayoutTests:

Updated the tests to reflect the rename of scoped to composed and the negation of its semantics.
Now every Event is assumed to be scoped / non-composed by default, and we need to explicitly set
composed to true in order for events to cross shadow boundaries.

Also, every Event behaves as if related target is assumed to be scoped in the old terminology
althoug the flag no longer exists.

* fast/shadow-dom/Extensions-to-Event-Interface-expected.txt:
* fast/shadow-dom/Extensions-to-Event-Interface.html: Removed a test case that was testing
relatedTargetScoped to false since this flag no longer exists.
* fast/shadow-dom/MouseEvent-prototype-offsetX-offsetY.html:
* fast/shadow-dom/event-inside-shadow-tree.html:
* fast/shadow-dom/event-inside-slotted-node.html:
* fast/shadow-dom/event-with-related-target.html:
* fast/shadow-dom/trusted-event-scoped-flags-expected.txt:
* fast/shadow-dom/trusted-event-scoped-flags.html:
* fast/xmlhttprequest/xmlhttprequest-get-expected.txt:
* http/tests/workers/worker-importScriptsOnError-expected.txt:
* inspector/model/remote-object-get-properties-expected.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@202953 268f45cc-cd09-0410-ab3c-d52691b4dbfc
matteoceraico pushed a commit to matteoceraico/webkit that referenced this issue Jul 5, 2017
… picker

https://bugs.webkit.org/show_bug.cgi?id=159686
Source/WebCore:

Reviewed by Chris Dumez.

The bug was caused by DOMActivate event not propagating out of the user-agent shadow tree
of a file input, and FileInputType not receiving the event to open the file picker.

Made DOMActivate "composed" event which cross shadow boundaries to fix the bug. The feedback
was given back to W3C on WICG/webcomponents#513 (comment)

Test: fast/forms/file/open-file-panel.html

* dom/Event.cpp:
(WebCore::Event::composed):

Tools:

Reviewed by Chris Dumez.

Added a code to print "OPEN FILE PANEL" in the text when runOpenPanel is called in the UI delegate.

* WebKitTestRunner/TestController.cpp:
(WTR::runOpenPanel):
(WTR::TestController::createOtherPage):
(WTR::TestController::createWebViewWithOptions):

LayoutTests:

<rdar://problem/27263589>

Reviewed by Chris Dumez.

Added a regression test for opening a file picker on a type=file input element.

The test currently only works on WebKit2 since the support for logging "OPEN FILE PANEL"
was only added to WebKitTestRunner.

Also added WebKit2 specific expected results for some tests that tries to open file panel.

* fast/forms/file/open-file-panel-expected.txt: Added.
* fast/forms/file/open-file-panel.html: Added.
* platform/ios-simulator-wk1/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/win/TestExpectations:
* platform/wk2/accessibility: Added.
* platform/wk2/accessibility/axpress-on-aria-button-expected.txt: Copied from LayoutTests/accessibility/axpress-on-aria-button-expected.txt.
* platform/wk2/accessibility/file-upload-button-with-axpress-expected.txt: Copied from LayoutTests/accessibility/file-upload-button-with-axpress-expected.txt.
* platform/wk2/fast: Added.
* platform/wk2/fast/events: Added.
* platform/wk2/fast/events/domactivate-sets-underlying-click-event-as-handled-expected.txt: Copied from LayoutTests/fast/events/domactivate-sets-underlying-click-event-as-handled-expected.txt.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@203187 268f45cc-cd09-0410-ab3c-d52691b4dbfc
matteoceraico pushed a commit to matteoceraico/webkit that referenced this issue Jul 5, 2017
https://bugs.webkit.org/show_bug.cgi?id=158415

Reviewed by Chris Dumez.

Source/WebCore:

Replace `scoped` flag with `composed` flag and negate its meaning per the latest spec:
https://dom.spec.whatwg.org/#dom-event-composed
WICG/webcomponents#513

In the old spec, every event was assumed to be "composed" (crosses shadow boundaries)
by default and there was `scoped` flag which prevented the event from crossing bondaries,
and there was a handful of events for which `scoped` was set true when dispatched by UA.

In the new spec, every event is assumed to be "scoped" and a handful of user-initiated
events set `composed` flag to true, which is also exposed in EventInit dictionary.
`relatedTargetScoped` flag has been removed. New behavior is identical to when this flag
was set to true.

No new tests since existing tests are updated to test the new flag and behavior.

* dom/CompositionEvent.cpp:
(WebCore::CompositionEvent::isCompositionEvent): Added.
* dom/CompositionEvent.h:
* dom/Event.cpp:
(WebCore::Event::Event): Initialize m_composed. Also re-ordered m_type and m_isInitialized
for better packing.
(WebCore::Event::composed): Renamed from Event::composed. We return true whenever composed
is set to true in EventInit, or the engine is dispatching an user-initiated event listed in:
WICG/webcomponents#513 (comment)
as well as keypress, cut, paste, and, copy as discussed in:
WICG/webcomponents#513 (comment)
(WebCore::Event::isCompositionEvent): Added.
* dom/Event.h:
(WebCore::Event::composed): Added.
(WebCore::Event::scoped): Deleted.
(WebCore::Event::relatedTargetScoped): Deleted.
(WebCore::Event): Reordered m_type and m_isInitialized for better packing. Added m_composed
and removed m_scoped and m_relatedTargetScoped.
* dom/Event.idl:
* dom/EventPath.cpp:
(WebCore::shouldEventCrossShadowBoundary): Returns true if the event did not originate from
a shadow tree (this event entered the current shadow tree via a slot so we need to proceed with
the normal bubble path outside the shadow tree) or composed flag is set true.
(WebCore::EventPath::EventPath): m_event no longer exists, which was only used to get the value
of relatedTargetScoped which has been removed.
(WebCore::EventPath::setRelatedTarget): Behave as if relatedTargetScoped is always set true
since the flag has been removed.
* dom/EventPath.h:
* dom/FocusEvent.cpp:
(WebCore::FocusEvent::relatedTargetScoped): Deleted.
* dom/FocusEvent.h:
* dom/MouseEvent.cpp:
(WebCore::MouseEvent::relatedTargetScoped): Deleted.
* dom/MouseEvent.h:

LayoutTests:

Updated the tests to reflect the rename of scoped to composed and the negation of its semantics.
Now every Event is assumed to be scoped / non-composed by default, and we need to explicitly set
composed to true in order for events to cross shadow boundaries.

Also, every Event behaves as if related target is assumed to be scoped in the old terminology
althoug the flag no longer exists.

* fast/shadow-dom/Extensions-to-Event-Interface-expected.txt:
* fast/shadow-dom/Extensions-to-Event-Interface.html: Removed a test case that was testing
relatedTargetScoped to false since this flag no longer exists.
* fast/shadow-dom/MouseEvent-prototype-offsetX-offsetY.html:
* fast/shadow-dom/event-inside-shadow-tree.html:
* fast/shadow-dom/event-inside-slotted-node.html:
* fast/shadow-dom/event-with-related-target.html:
* fast/shadow-dom/trusted-event-scoped-flags-expected.txt:
* fast/shadow-dom/trusted-event-scoped-flags.html:
* fast/xmlhttprequest/xmlhttprequest-get-expected.txt:
* http/tests/workers/worker-importScriptsOnError-expected.txt:
* inspector/model/remote-object-get-properties-expected.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@202953 268f45cc-cd09-0410-ab3c-d52691b4dbfc
matteoceraico pushed a commit to matteoceraico/webkit that referenced this issue Jul 5, 2017
… picker

https://bugs.webkit.org/show_bug.cgi?id=159686
Source/WebCore:

Reviewed by Chris Dumez.

The bug was caused by DOMActivate event not propagating out of the user-agent shadow tree
of a file input, and FileInputType not receiving the event to open the file picker.

Made DOMActivate "composed" event which cross shadow boundaries to fix the bug. The feedback
was given back to W3C on WICG/webcomponents#513 (comment)

Test: fast/forms/file/open-file-panel.html

* dom/Event.cpp:
(WebCore::Event::composed):

Tools:

Reviewed by Chris Dumez.

Added a code to print "OPEN FILE PANEL" in the text when runOpenPanel is called in the UI delegate.

* WebKitTestRunner/TestController.cpp:
(WTR::runOpenPanel):
(WTR::TestController::createOtherPage):
(WTR::TestController::createWebViewWithOptions):

LayoutTests:

<rdar://problem/27263589>

Reviewed by Chris Dumez.

Added a regression test for opening a file picker on a type=file input element.

The test currently only works on WebKit2 since the support for logging "OPEN FILE PANEL"
was only added to WebKitTestRunner.

Also added WebKit2 specific expected results for some tests that tries to open file panel.

* fast/forms/file/open-file-panel-expected.txt: Added.
* fast/forms/file/open-file-panel.html: Added.
* platform/ios-simulator-wk1/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/win/TestExpectations:
* platform/wk2/accessibility: Added.
* platform/wk2/accessibility/axpress-on-aria-button-expected.txt: Copied from LayoutTests/accessibility/axpress-on-aria-button-expected.txt.
* platform/wk2/accessibility/file-upload-button-with-axpress-expected.txt: Copied from LayoutTests/accessibility/file-upload-button-with-axpress-expected.txt.
* platform/wk2/fast: Added.
* platform/wk2/fast/events: Added.
* platform/wk2/fast/events/domactivate-sets-underlying-click-event-as-handled-expected.txt: Copied from LayoutTests/fast/events/domactivate-sets-underlying-click-event-as-handled-expected.txt.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@203187 268f45cc-cd09-0410-ab3c-d52691b4dbfc
matteoceraico pushed a commit to matteoceraico/webkit that referenced this issue Jul 6, 2017
https://bugs.webkit.org/show_bug.cgi?id=158415

Reviewed by Chris Dumez.

Source/WebCore:

Replace `scoped` flag with `composed` flag and negate its meaning per the latest spec:
https://dom.spec.whatwg.org/#dom-event-composed
WICG/webcomponents#513

In the old spec, every event was assumed to be "composed" (crosses shadow boundaries)
by default and there was `scoped` flag which prevented the event from crossing bondaries,
and there was a handful of events for which `scoped` was set true when dispatched by UA.

In the new spec, every event is assumed to be "scoped" and a handful of user-initiated
events set `composed` flag to true, which is also exposed in EventInit dictionary.
`relatedTargetScoped` flag has been removed. New behavior is identical to when this flag
was set to true.

No new tests since existing tests are updated to test the new flag and behavior.

* dom/CompositionEvent.cpp:
(WebCore::CompositionEvent::isCompositionEvent): Added.
* dom/CompositionEvent.h:
* dom/Event.cpp:
(WebCore::Event::Event): Initialize m_composed. Also re-ordered m_type and m_isInitialized
for better packing.
(WebCore::Event::composed): Renamed from Event::composed. We return true whenever composed
is set to true in EventInit, or the engine is dispatching an user-initiated event listed in:
WICG/webcomponents#513 (comment)
as well as keypress, cut, paste, and, copy as discussed in:
WICG/webcomponents#513 (comment)
(WebCore::Event::isCompositionEvent): Added.
* dom/Event.h:
(WebCore::Event::composed): Added.
(WebCore::Event::scoped): Deleted.
(WebCore::Event::relatedTargetScoped): Deleted.
(WebCore::Event): Reordered m_type and m_isInitialized for better packing. Added m_composed
and removed m_scoped and m_relatedTargetScoped.
* dom/Event.idl:
* dom/EventPath.cpp:
(WebCore::shouldEventCrossShadowBoundary): Returns true if the event did not originate from
a shadow tree (this event entered the current shadow tree via a slot so we need to proceed with
the normal bubble path outside the shadow tree) or composed flag is set true.
(WebCore::EventPath::EventPath): m_event no longer exists, which was only used to get the value
of relatedTargetScoped which has been removed.
(WebCore::EventPath::setRelatedTarget): Behave as if relatedTargetScoped is always set true
since the flag has been removed.
* dom/EventPath.h:
* dom/FocusEvent.cpp:
(WebCore::FocusEvent::relatedTargetScoped): Deleted.
* dom/FocusEvent.h:
* dom/MouseEvent.cpp:
(WebCore::MouseEvent::relatedTargetScoped): Deleted.
* dom/MouseEvent.h:

LayoutTests:

Updated the tests to reflect the rename of scoped to composed and the negation of its semantics.
Now every Event is assumed to be scoped / non-composed by default, and we need to explicitly set
composed to true in order for events to cross shadow boundaries.

Also, every Event behaves as if related target is assumed to be scoped in the old terminology
althoug the flag no longer exists.

* fast/shadow-dom/Extensions-to-Event-Interface-expected.txt:
* fast/shadow-dom/Extensions-to-Event-Interface.html: Removed a test case that was testing
relatedTargetScoped to false since this flag no longer exists.
* fast/shadow-dom/MouseEvent-prototype-offsetX-offsetY.html:
* fast/shadow-dom/event-inside-shadow-tree.html:
* fast/shadow-dom/event-inside-slotted-node.html:
* fast/shadow-dom/event-with-related-target.html:
* fast/shadow-dom/trusted-event-scoped-flags-expected.txt:
* fast/shadow-dom/trusted-event-scoped-flags.html:
* fast/xmlhttprequest/xmlhttprequest-get-expected.txt:
* http/tests/workers/worker-importScriptsOnError-expected.txt:
* inspector/model/remote-object-get-properties-expected.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@202953 268f45cc-cd09-0410-ab3c-d52691b4dbfc
matteoceraico pushed a commit to matteoceraico/webkit that referenced this issue Jul 6, 2017
… picker

https://bugs.webkit.org/show_bug.cgi?id=159686
Source/WebCore:

Reviewed by Chris Dumez.

The bug was caused by DOMActivate event not propagating out of the user-agent shadow tree
of a file input, and FileInputType not receiving the event to open the file picker.

Made DOMActivate "composed" event which cross shadow boundaries to fix the bug. The feedback
was given back to W3C on WICG/webcomponents#513 (comment)

Test: fast/forms/file/open-file-panel.html

* dom/Event.cpp:
(WebCore::Event::composed):

Tools:

Reviewed by Chris Dumez.

Added a code to print "OPEN FILE PANEL" in the text when runOpenPanel is called in the UI delegate.

* WebKitTestRunner/TestController.cpp:
(WTR::runOpenPanel):
(WTR::TestController::createOtherPage):
(WTR::TestController::createWebViewWithOptions):

LayoutTests:

<rdar://problem/27263589>

Reviewed by Chris Dumez.

Added a regression test for opening a file picker on a type=file input element.

The test currently only works on WebKit2 since the support for logging "OPEN FILE PANEL"
was only added to WebKitTestRunner.

Also added WebKit2 specific expected results for some tests that tries to open file panel.

* fast/forms/file/open-file-panel-expected.txt: Added.
* fast/forms/file/open-file-panel.html: Added.
* platform/ios-simulator-wk1/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/win/TestExpectations:
* platform/wk2/accessibility: Added.
* platform/wk2/accessibility/axpress-on-aria-button-expected.txt: Copied from LayoutTests/accessibility/axpress-on-aria-button-expected.txt.
* platform/wk2/accessibility/file-upload-button-with-axpress-expected.txt: Copied from LayoutTests/accessibility/file-upload-button-with-axpress-expected.txt.
* platform/wk2/fast: Added.
* platform/wk2/fast/events: Added.
* platform/wk2/fast/events/domactivate-sets-underlying-click-event-as-handled-expected.txt: Copied from LayoutTests/fast/events/domactivate-sets-underlying-click-event-as-handled-expected.txt.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@203187 268f45cc-cd09-0410-ab3c-d52691b4dbfc
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=158415

Reviewed by Chris Dumez.

Source/WebCore:

Replace `scoped` flag with `composed` flag and negate its meaning per the latest spec:
https://dom.spec.whatwg.org/#dom-event-composed
WICG/webcomponents#513

In the old spec, every event was assumed to be "composed" (crosses shadow boundaries)
by default and there was `scoped` flag which prevented the event from crossing bondaries,
and there was a handful of events for which `scoped` was set true when dispatched by UA.

In the new spec, every event is assumed to be "scoped" and a handful of user-initiated
events set `composed` flag to true, which is also exposed in EventInit dictionary.
`relatedTargetScoped` flag has been removed. New behavior is identical to when this flag
was set to true.

No new tests since existing tests are updated to test the new flag and behavior.

* dom/CompositionEvent.cpp:
(WebCore::CompositionEvent::isCompositionEvent): Added.
* dom/CompositionEvent.h:
* dom/Event.cpp:
(WebCore::Event::Event): Initialize m_composed. Also re-ordered m_type and m_isInitialized
for better packing.
(WebCore::Event::composed): Renamed from Event::composed. We return true whenever composed
is set to true in EventInit, or the engine is dispatching an user-initiated event listed in:
WICG/webcomponents#513 (comment)
as well as keypress, cut, paste, and, copy as discussed in:
WICG/webcomponents#513 (comment)
(WebCore::Event::isCompositionEvent): Added.
* dom/Event.h:
(WebCore::Event::composed): Added.
(WebCore::Event::scoped): Deleted.
(WebCore::Event::relatedTargetScoped): Deleted.
(WebCore::Event): Reordered m_type and m_isInitialized for better packing. Added m_composed
and removed m_scoped and m_relatedTargetScoped.
* dom/Event.idl:
* dom/EventPath.cpp:
(WebCore::shouldEventCrossShadowBoundary): Returns true if the event did not originate from
a shadow tree (this event entered the current shadow tree via a slot so we need to proceed with
the normal bubble path outside the shadow tree) or composed flag is set true.
(WebCore::EventPath::EventPath): m_event no longer exists, which was only used to get the value
of relatedTargetScoped which has been removed.
(WebCore::EventPath::setRelatedTarget): Behave as if relatedTargetScoped is always set true
since the flag has been removed.
* dom/EventPath.h:
* dom/FocusEvent.cpp:
(WebCore::FocusEvent::relatedTargetScoped): Deleted.
* dom/FocusEvent.h:
* dom/MouseEvent.cpp:
(WebCore::MouseEvent::relatedTargetScoped): Deleted.
* dom/MouseEvent.h:

LayoutTests:

Updated the tests to reflect the rename of scoped to composed and the negation of its semantics.
Now every Event is assumed to be scoped / non-composed by default, and we need to explicitly set
composed to true in order for events to cross shadow boundaries.

Also, every Event behaves as if related target is assumed to be scoped in the old terminology
althoug the flag no longer exists.

* fast/shadow-dom/Extensions-to-Event-Interface-expected.txt:
* fast/shadow-dom/Extensions-to-Event-Interface.html: Removed a test case that was testing
relatedTargetScoped to false since this flag no longer exists.
* fast/shadow-dom/MouseEvent-prototype-offsetX-offsetY.html:
* fast/shadow-dom/event-inside-shadow-tree.html:
* fast/shadow-dom/event-inside-slotted-node.html:
* fast/shadow-dom/event-with-related-target.html:
* fast/shadow-dom/trusted-event-scoped-flags-expected.txt:
* fast/shadow-dom/trusted-event-scoped-flags.html:
* fast/xmlhttprequest/xmlhttprequest-get-expected.txt:
* http/tests/workers/worker-importScriptsOnError-expected.txt:
* inspector/model/remote-object-get-properties-expected.txt:


Canonical link: https://commits.webkit.org/177677@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202953 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
… picker

https://bugs.webkit.org/show_bug.cgi?id=159686
Source/WebCore:


Reviewed by Chris Dumez.

The bug was caused by DOMActivate event not propagating out of the user-agent shadow tree
of a file input, and FileInputType not receiving the event to open the file picker.

Made DOMActivate "composed" event which cross shadow boundaries to fix the bug. The feedback
was given back to W3C on WICG/webcomponents#513 (comment)

Test: fast/forms/file/open-file-panel.html

* dom/Event.cpp:
(WebCore::Event::composed):

Tools:


Reviewed by Chris Dumez.

Added a code to print "OPEN FILE PANEL" in the text when runOpenPanel is called in the UI delegate.

* WebKitTestRunner/TestController.cpp:
(WTR::runOpenPanel):
(WTR::TestController::createOtherPage):
(WTR::TestController::createWebViewWithOptions):

LayoutTests:

<rdar://problem/27263589>

Reviewed by Chris Dumez.

Added a regression test for opening a file picker on a type=file input element.

The test currently only works on WebKit2 since the support for logging "OPEN FILE PANEL"
was only added to WebKitTestRunner.

Also added WebKit2 specific expected results for some tests that tries to open file panel.

* fast/forms/file/open-file-panel-expected.txt: Added.
* fast/forms/file/open-file-panel.html: Added.
* platform/ios-simulator-wk1/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/win/TestExpectations:
* platform/wk2/accessibility: Added.
* platform/wk2/accessibility/axpress-on-aria-button-expected.txt: Copied from LayoutTests/accessibility/axpress-on-aria-button-expected.txt.
* platform/wk2/accessibility/file-upload-button-with-axpress-expected.txt: Copied from LayoutTests/accessibility/file-upload-button-with-axpress-expected.txt.
* platform/wk2/fast: Added.
* platform/wk2/fast/events: Added.
* platform/wk2/fast/events/domactivate-sets-underlying-click-event-as-handled-expected.txt: Copied from LayoutTests/fast/events/domactivate-sets-underlying-click-event-as-handled-expected.txt.


Canonical link: https://commits.webkit.org/177879@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@203187 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants