1823757 - implement PopoverInvokerElement interface

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Comment 1

[alexander :surkov (:asurkov)]

Attachment: Bug 1823757 - add PopoverInvokerElement interface

Comment 2

[alexander :surkov (:asurkov)]

Attachment: Bug 1823757 - implement attr associated element, r?emilio

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69eb90a97279
add PopoverInvokerElement interface r=emilio

Comment 4

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/69eb90a97279

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7301e6523b60
implement attr associated element, r=emilio

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/7301e6523b60

Comment 7

[alexander :surkov (:asurkov)]

Attachment: Bug 1823757 implement PopoverInvokerElement::popoverTargetElement, r?emilio

See https://html.spec.whatwg.org/multipage/popover.html#dom-popovertargetelement

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75f2676b3aae
implement PopoverInvokerElement::popoverTargetElement, r=emilio

Comment 9

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/75f2676b3aae

Comment 10

[alexander :surkov (:asurkov)]

Attachment: Bug 1823757 - use parsed atoms for popovertarget attribute, r?emilio

Use parsed atoms to store popovertarget attributes as more effective mechanism to search for ID match in attr associated elements implementation. Followup from D173337.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93a5b0a259ae
use parsed atoms for popovertarget attribute, r=emilio

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e952bc125116
Backed out changeset 93a5b0a259ae for causing Atom related wpt failures.

Comment 13

[Cosmin Sabou [:CosminS]]

Backed out for causing Atom related wpt failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=93a5b0a259ae332b9af74d543d347d6e8d383c10&tochange=95e453c44af9440d05ffb65b1a2878694cb20564&selectedTaskRun=P8JK_AQ-SF2c7AfcUy1l8g.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=411640355&repo=autoland&lineNumber=12790

Backout link: https://hg.mozilla.org/integration/autoland/rev/e952bc1251164d5c3816b2d37bca28fd2ab4f5e6

Comment 14

[alexander :surkov (:asurkov)]

It crashes because of assert:
Mozilla crash reason: MOZ_ASSERT(Type() == eAtom) (wrong type)

Element::GetAttrAssociatedElement triggered for popovertarget attribute. The problem is nsGenericHTMLFormControlElementWithState::ParseAttribute() never triggers and thus popovertarget is not parsed. However I see that Element::SetAttr (https://searchfox.org/mozilla-central/source/dom/base/Element.cpp#2498) triggers ParseAttribute() for nsGenericHTMLFormControlElementWithState element for popovertarget attribute.

I didn't yet manage to setup a debug env on my mac, so I didn't manage to run it through a debugger yet, but just in case, Emilio, do I miss anything evident here?

Comment 15

[alexander :surkov (:asurkov)]

the problem is HTMLButtonElement::ParseAttribute doesn't call into nsGenericHTMLFormControlElementWithState

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/96da9dcee2b0
use parsed atoms for popovertarget attribute, r=emilio

Comment 17

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on popover-invoking-attribute.html

• backout: https://hg.mozilla.org/integration/autoland/rev/9bdc51a260ca5f69d33c95d2cc8d9ddf33e7bb0e
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=Rg9FQhTqQmKmk-WSeHw4oA.0&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&revision=96da9dcee2b0061b45fb0f926567dfe3acd54adf
• backfills: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&fromchange=4a13bf07d9cd5b2c8239adabe60022798651ae50&searchStr=os%2Cx%2C10.15%2Cwebrender%2Copt%2Cweb%2Cplatform%2Ctests%2Ctest-macosx1015-64-qr%2Fopt-web-platform-tests%2Cwpt10&group_state=expanded&tochange=97295a57babe545a8cdd564648c669e2a82c9825
• failure log: https://treeherder.mozilla.org/logviewer?job_id=411939835&repo=autoland&lineNumber=9742

[task 2023-04-11T06:28:29.459Z] 06:28:29     INFO - TEST-FAIL | /html/semantics/popovers/popover-invoking-attribute.html | Test <input type="image">, action=undefined, popoverTarget IDL, popoverTargetAction IDL, with popover=auto - assert_true: Toggle or show should show the popover expected true got false
[task 2023-04-11T06:28:29.459Z] 06:28:29     INFO - @http://web-platform.test:8000/html/semantics/popovers/popover-invoking-attribute.html:100:30
[task 2023-04-11T06:28:29.459Z] 06:28:29     INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-invoking-attribute.html | Test <input type="text">, action=toggle, popovertarget attr, popovertargetaction attr, with popover=auto - expected FAIL
[task 2023-04-11T06:28:29.459Z] 06:28:29     INFO - TEST-INFO | expected FAIL
[task 2023-04-11T06:28:29.460Z] 06:28:29     INFO - 
[task 2023-04-11T06:28:29.460Z] 06:28:29     INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-invoking-attribute.html | Test <input type="text">, action=toggle, popovertarget attr, popoverTargetAction IDL, with popover=auto - expected FAIL
[task 2023-04-11T06:28:29.460Z] 06:28:29     INFO - TEST-INFO | expected FAIL
[task 2023-04-11T06:28:29.460Z] 06:28:29     INFO - 
[task 2023-04-11T06:28:29.460Z] 06:28:29     INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-invoking-attribute.html | Test <input type="text">, action=toggle, popoverTarget IDL, popovertargetaction attr, with popover=auto - expected FAIL
[task 2023-04-11T06:28:29.460Z] 06:28:29     INFO - TEST-INFO | expected FAIL
[task 2023-04-11T06:28:29.461Z] 06:28:29     INFO - 
[task 2023-04-11T06:28:29.461Z] 06:28:29     INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-invoking-attribute.html | Test <input type="text">, action=toggle, popoverTarget IDL, popoverTargetAction IDL, with popover=auto - expected FAIL
[task 2023-04-11T06:28:29.461Z] 06:28:29     INFO - TEST-INFO | expected FAIL
[task 2023-04-11T06:28:29.462Z] 06:28:29     INFO - 
[task 2023-04-11T06:28:29.462Z] 06:28:29     INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-invoking-attribute.html | Test <input type="text">, action=hide, popovertarget attr, popovertargetaction attr, with popover=auto - expected FAIL
[task 2023-04-11T06:28:29.462Z] 06:28:29     INFO - TEST-INFO | expected FAIL
[task 2023-04-11T06:28:29.462Z] 06:28:29     INFO - 

Comment 18

[alexander :surkov (:asurkov)]

It's weird that the patch breaks tests (wondering why ./mach try auto didn't catch those) because it wasn't supposed to change behavior, it's good though all of failures are unexpected passes, so I think I will simply update expectations.

Comment 19

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov (:asurkov) from comment #18)

It's weird that the patch breaks tests (wondering why ./mach try auto didn't catch those) because it wasn't supposed to change behavior, it's good though all of failures are unexpected passes, so I think I will simply update expectations.

ah, it's the same issue as in comment #15, thus popovertargetaction was not parsed previously, now it does and thus make tests passing

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc54299fdf1d
use parsed atoms for popovertarget attribute, r=emilio

Comment 21

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/fc54299fdf1d

Comment 22

[Hamish Willee]

MDN docs work for this can be tracked in https://github.com/mdn/content/issues/26694 (mostly done).