Integrated activation behaviour into event dispatch algorithm #22981

aditj · 2019-03-06T18:30:09Z

Trying to update the synthetic mouse click algorithm according to the spec

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #22783 (GitHub issue number if applicable)

There are tests for these changes OR
These changes do not require tests because ___

This change is

highfive · 2019-03-06T18:30:17Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/document.rs, components/script/dom/activation.rs, components/script/dom/performance.rs, components/script/dom/event.rs, components/script/dom/webidls/MouseEvent.webidl and 2 more
@KiChjang: components/script/dom/document.rs, components/script/dom/activation.rs, components/script/dom/performance.rs, components/script/dom/event.rs, components/script/dom/webidls/MouseEvent.webidl and 2 more

highfive · 2019-03-06T18:30:20Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

aditj · 2019-03-06T18:31:33Z

Help Please! How can I remove the previously approved commit from this pull? :D

jdm · 2019-03-06T18:40:27Z

I recommend doing this:

$ git checkout aditj-patch-6
$ git reset --hard de09c8fb6ef87dec5932d5fab4adcb421d291a54
$ git cherry-pick 009db7396362dc4febba4f5bb43d08cd0e6feb9e
$ git push -f origin aditj-patch-6

Manishearth · 2019-03-11T17:26:52Z

components/script/dom/activation.rs

    target.dispatch_event(event);
-
-    // Step 5
+    event.dispatching();


this does nothing, please remove this line

the spec asks us to return the result of dispatching, that would mean returning the EventStatus returned by dispatch_event (however I don't see anything in the spec that uses this right now)

Manishearth · 2019-03-11T17:26:52Z

components/script/dom/event.rs

@@ -169,6 +170,9 @@ impl Event {
        // Step 3-4.
        let path = self.construct_event_path(&target);
        rooted_vec!(let event_path <- path.into_iter());
+        if self.type_() == atom!("click") {
+            is_activation_event = true;


This variable isn't used, also you can fire manual click events that aren't activation events. Please remove the is_activation_event stuff.

I think is_activition_event will be used in Steps 9.6.1 and Steps 9.8.1 of the spec

Ah, I see. Can you implement that part?

@Manishearth I can certainly try!
But how do I check if an event target has Activation behaviour?

Downcast it to an element, and if that succeeds, call .as_maybe_activatable() on it.

Manishearth · 2019-03-11T17:26:52Z

components/script/dom/activation.rs

-    // Step 2
-    element.set_click_in_progress(true);
-    // Step 3
+    // Step 12 of https://dom.spec.whatwg.org/#concept-event-dispatch
    let activatable = element.as_maybe_activatable();
    if let Some(a) = activatable {
        a.pre_click_activation();


As mentioned in @jdm's comment, this stuff needs to be moved into the event dispatch code

https://dom.spec.whatwg.org/#concept-event-dispatch

I'll remove this from here.
Coming to implementation in the event dispatch code there's a comment mentioning step 12 of the spec . Although I am unsure of the correctness of this implementation.


        Changed the Firing a Synthetic MouseEvent Algorithm

jdm · 2019-03-18T20:48:51Z

@Manishearth Re-review ping.

Manishearth · 2019-03-18T20:55:21Z

@jdm The requested changes haven't yet been made

bors-servo · 2019-10-23T03:42:39Z

☔ The latest upstream changes (presumably #24489) made this pull request unmergeable. Please resolve the merge conflicts.

jdm · 2020-01-08T03:11:11Z

Closing due to inactivity.

highfive assigned Manishearth Mar 6, 2019

highfive added the S-awaiting-review label Mar 6, 2019

aditj force-pushed the aditj:aditj-patch-6 branch 5 times, most recently from 6c090b2 to d0ac174 Mar 6, 2019

Manishearth requested changes Mar 11, 2019

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Mar 11, 2019

aditj force-pushed the aditj:aditj-patch-6 branch from d0ac174 to ef8c977 Mar 14, 2019

highfive added S-awaiting-review and removed S-needs-code-changes labels Mar 14, 2019

aditj force-pushed the aditj:aditj-patch-6 branch 2 times, most recently from ba5a291 to 3493bd5 Mar 14, 2019

Changed the Firing a Synthetic MouseEvent Algorithm

Loading status checks…

f132ad3

aditj force-pushed the aditj:aditj-patch-6 branch 3 times, most recently from 1f68409 to f132ad3 Mar 14, 2019

Manishearth added S-needs-code-changes and removed S-awaiting-review labels Jun 14, 2019

highfive added the S-needs-rebase label Oct 23, 2019

jdm closed this Jan 8, 2020

jdm mentioned this pull request Jan 8, 2020

Integrate activation behaviour into event dispatch algorithm #22783

Closed

servo / servo

Integrated activation behaviour into event dispatch algorithm #22981

Integrated activation behaviour into event dispatch algorithm #22981

aditj commented Mar 6, 2019 •

edited by CYBAI

highfive commented Mar 6, 2019

highfive commented Mar 6, 2019

aditj commented Mar 6, 2019

jdm commented Mar 6, 2019 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jdm commented Mar 18, 2019

Manishearth commented Mar 18, 2019

bors-servo commented Oct 23, 2019

jdm commented Jan 8, 2020

servo / servo

Join GitHub today

Integrated activation behaviour into event dispatch algorithm #22981

Integrated activation behaviour into event dispatch algorithm #22981

Conversation

aditj commented Mar 6, 2019 • edited by CYBAI

highfive commented Mar 6, 2019

highfive commented Mar 6, 2019

aditj commented Mar 6, 2019

jdm commented Mar 6, 2019 • edited

This comment has been minimized.

Manishearth Mar 11, 2019

This comment has been minimized.

Manishearth Mar 11, 2019

This comment has been minimized.

aditj Mar 11, 2019

This comment has been minimized.

Manishearth Mar 11, 2019

This comment has been minimized.

aditj Mar 12, 2019 • edited

This comment has been minimized.

Manishearth Mar 21, 2019

This comment has been minimized.

Manishearth Mar 11, 2019

This comment has been minimized.

aditj Mar 12, 2019

jdm commented Mar 18, 2019

Manishearth commented Mar 18, 2019

bors-servo commented Oct 23, 2019

jdm commented Jan 8, 2020

aditj commented Mar 6, 2019 •

edited by CYBAI

jdm commented Mar 6, 2019 •

edited

aditj Mar 12, 2019 •

edited