1851970 - Use EventTarget::ActivationBehavior for more elements

Status: ASSIGNED

(Core :: DOM: Events, enhancement, P2)

Description

[Vincent Hilla [:vhilla]]

Bug 1658996 introduced new methods around activation behavior and applied them to several input elements to fix wpt failures.

In a follow up, these methods should be used anywhere where elements exhibit activation behavior. Where possible, landed separately.

Comment 1

[Vincent Hilla [:vhilla]]

Attachment: Bug 1851970 - Part 1: Use activation behavior for submit and image input type. r=edgar

Comment 2

[Vincent Hilla [:vhilla]]

Attachment: Bug 1851970 - Part 2: Activation behavior method for button element. r=edgar

Comment 3

[Vincent Hilla [:vhilla]]

Attachment: Bug 1851970 - Part 1: Activation behavior method for label. r=edgar

Comment 4

[Vincent Hilla [:vhilla]]

Attachment: Bug 1851970 - Part 2: Activation behavior method for links. r=edgar

Comment 5

[Vincent Hilla [:vhilla]]

Attachment: Bug 1851970 - Part 3: Activation behavior method for summary element. r=edgar

Comment 6

[Vincent Hilla [:vhilla]]

:edgar are we confident enough to land all 5 parts together? We could also land the changes to form submission, i.e. parts 1 and 2, separately.

In that case, I can modify the patches so that we don't get temporary failures in Event-dispatch-single-activation-behavior.html. That would probably mean to keep setting aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault during PostHandleEvent of input and button elements and removing this in part 3 instead of 1 and 2.

Comment 7

[Edgar Chen [:edgar]]

If we could only land the changes to form submission first without breaking other element's behavior, that would be great.

Comment 8

[Phabricator Automation]

Comment on attachment 9351959 [details]
Bug 1851970 - Part 1: Use activation behavior for submit and image input type. r=edgar

Revision D187185 was moved to bug 1855633. Setting attachment 9351959 [details] to obsolete.

Comment 9

[Phabricator Automation]

Comment on attachment 9351960 [details]
Bug 1851970 - Part 2: Activation behavior method for button element. r=edgar

Revision D183989 was moved to bug 1855633. Setting attachment 9351960 [details] to obsolete.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:vhilla, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 11

[Edgar Chen [:edgar]]

If my understanding is correct, I think we would like to land bug 1855633 first.

Comment 13

[Vincent Hilla [:vhilla]]

:edgar I think this is ready to land, do you agree?

Comment 14

[Edgar Chen [:edgar]]

Yes, let's land this, thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b99a620a535c
Part 1: Activation behavior method for label. r=edgar
https://hg.mozilla.org/integration/autoland/rev/23ddc229d1a1
Part 2: Activation behavior method for links. r=edgar
https://hg.mozilla.org/integration/autoland/rev/a2536c6c6c23
Part 3: Activation behavior method for summary element. r=edgar

Comment 16

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in browser_aboutNetError_csp_iframe.js.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/base/content/test/about/browser_aboutNetError_csp_iframe.js | Test timed out -

Comment 17

[Vincent Hilla [:vhilla]]

The aboutNetError page has a button element within a link and expects the link to be activated once the button is pressed, see here.

Example

<a href="#success"><button>click here</button></a>

I have to re-visit the spec whether this is the intended behavior. Though button has activation behavior and will be the first element in the event target chain, thus should become the unique activation target.

Chromium would activate the link and this pattern looks like something many pages might use, so changing this behavior is probably too risky.

Comment 18

[Vincent Hilla [:vhilla]]

The content model of an a element does not allow nesting a button. And if it is done, the dispatch algorithm should only invoke activation behavior on the button.

Though there is this stackoverflow question about it that received much attention and browsers do currently activate both the button and a element. Having a link look like a button seems to be a common use case.

:zcorpan, can you comment on this?

Comment 19

[Simon Pieters [:zcorpan]]

It seems whether the link is followed (as implemented in WebKit/Chromium/Gecko, based on testing but not looking at the code) depends on whether the activation behavior for the button returns without doing anything. In particular, if there is a form owner, the button submits the form and the link is not followed:

https://html.spec.whatwg.org/#the-button-element:activation-behaviour

Per spec, I think since the button element has a defined activation behavior, it should be the activation target. But this is likely a spec bug. I suppose the activation behavior steps could return a boolean value to indicate whether the activation behavior did anything (if that matches how it's implemented). Or maybe there should be no activation behavior if there's no form owner?

I found this spec issue by :smaug https://github.com/whatwg/html/issues/1567 - NI to comment on how this is implemented in Gecko.

Comment 20

[Vincent Hilla [:vhilla]]

(In reply to Simon Pieters [:zcorpan] from comment #19)

I suppose the activation behavior steps could return a boolean value to indicate whether the activation behavior did anything (if that matches how it's implemented). Or maybe there should be no activation behavior if there's no form owner?

A boolean value would not work, as per the dispatch algorithm (and similar implementation), activation behavior executes in step 11 were it would be too late to determine the next activation target.
Maybe the spec should state that a button has activation behavior if

• it is disabled
• or has a form owner and is not in type=button
  From testing, a disabled button prevents activation behavior on it's ancestor elements by becoming activation target itself and doing nothing.

Comment 21

[Vincent Hilla [:vhilla]]

Though that would not work with summary elements where a plain button does prevent activation behavior on the ancestor. E.g.

data:text/html,<details><summary>Summary<button>Btn</button></summary>Text</details>

In gecko, we enforce that interactive content descendants of a summary element cannot activate this summary. This could be added to the spec.

Comment 22

[Vincent Hilla [:vhilla]]

(In reply to Vincent Hilla [:vhilla] from comment #20)

Maybe the spec should state that a button has activation behavior if

• it is disabled
• or has a form owner and is not in type=button

Or has a popovertarget

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

This was discussed in the spec issue

Comment 24

[Vincent Hilla [:vhilla]]

Attachment: WIP: Bug 1851970 - Part 4: Alternative activation behavior model. r=edgar

Depends on D183992