1055773 - Move hasAttributes() to Element

Status: RESOLVED FIXED

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

Description

[Anne (:annevk)]

It seems Chrome successfully moved these from node to element. So should we.

Prior: bug 849661 and bug 856752.

Comment 1

[Arkadiusz Michalski (Spirit)]

"Since Firefox 22, Node.attributes is no longer supported (not implemented by other browsers and removed from the spec). It is only supported on Element (Element.attributes)."

https://developer.mozilla.org/en-US/docs/Web/API/Node.attributes

Comment 2

[Nick Lebedev [:nl]]

Attachment: Move hasAttributes method from Node to Element.

Could you take a look at my patch please.

I have two concerns regarding my changes:
1. Whether i did correct changes to the .idl & .webidl files.
2. Whether i updated all the required tests.

Comment 3

[Nick Lebedev [:nl]]

I was also making two try runs: first where i forget to make change in the test_historical.html : https://tbpl.mozilla.org/?tree=Try&rev=515980ec2f27 and second after i did it: https://tbpl.mozilla.org/?tree=Try&rev=46557c5ebc78.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8489255 [details] [diff] [review]
Move hasAttributes method from Node to Element.

You need to change the IIDs of the two XPIDL interfaces.  r=me with that.

Comment 5

[Boris Zbarsky [:bzbarsky]]

ms2ger, note the imptests change here.

Comment 6

[Nick Lebedev [:nl]]

Attachment: Move hasAttributes method from Node to Element. v2.

Updated uuids according to bz's comment.

Comment 7

[Carsten Book [:Tomcat]]

Hi, patch failed to apply cleanly:

patching file content/base/public/Element.h
Hunk #1 FAILED at 640
1 out of 3 hunks FAILED -- saving rejects to file content/base/public/Element.h.rej
patching file dom/webidl/Element.webidl
Hunk #1 FAILED at 45
1 out of 1 hunks FAILED -- saving rejects to file dom/webidl/Element.webidl.rej

could you take a look and maybe rebase? thanks!

Comment 8

[Nick Lebedev [:nl]]

Attachment: Move hasAttributes method from Node to Element. v3.

Oops, sorry. Rebased.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5850ae46157b

And a follow-up for other UUID bumps that |mach update-uuids nsIDOMElement nsIDOMNode| say need updating:
https://hg.mozilla.org/integration/mozilla-inbound/rev/72b7c1efb2b5

Comment 10

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f4123c7172ee - debug mochitest-3 (hmm, do we only run dom-level2-core on debug?) wasn't happy, https://tbpl.mozilla.org/php/getParsedLog.php?id=48259643&tree=Mozilla-Inbound, nor was web-platform-tests-2, https://tbpl.mozilla.org/php/getParsedLog.php?id=48257612&tree=Mozilla-Inbound

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

The imptests change is wrong; /dom/imptests/failures/html/dom/test_historical.html.json and  /testing/web-platform/meta/dom/historical.html.ini need to be updated to reflect that we now pass the test.

Comment 12

[Nick Lebedev [:nl]]

Could you please clarify how I should update the following files:

1. Here I need simply remove the following line,
http://mxr.mozilla.org/mozilla-central/source/dom/imptests/failures/html/dom/test_historical.html.json#12
Am I correct?

2. Should I simply remove the following line too? Or change it to PASS?
http://mxr.mozilla.org/mozilla-central/source/testing/web-platform/meta/dom/historical.html.ini#33

3. How would you advice to fix the failure on this line:
http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/dom-level2-core/test_nodehasattributes02.html?force=1#99

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

1. yes, and don't forget to remove the trailing comma from the previous line
2. just remove those three lines
3. add the name of the test to dom/tests/mochitest/dom-level2-core/exclusions.js; I guess attrExodus will do.

Comment 14

[Nick Lebedev [:nl]]

Attachment: has_attr

Try results for all mochi/webplatform tests:
https://tbpl.mozilla.org/?tree=Try&rev=56b464015291. It contains one small issue which i've fixed and it is reflected in the second try results: https://tbpl.mozilla.org/?tree=Try&rev=dfc446a1846b.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8498150 [details] [diff] [review]
has_attr

>+++ b/dom/imptests/html/dom/test_historical.html

Why is the change to this file still needed?  I don't think it should be.

r=me with that change removed.

Comment 16

[Nick Lebedev [:nl]]

Attachment: Move hasAttributes method from Node to Element. v5.

Done.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3fc41feeb22

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[Jean-Yves Perrier [:teoli]]

Updated:
https://developer.mozilla.org/en-US/docs/Web/API/Element.hasAttributes
and
https://developer.mozilla.org/en-US/Firefox/Releases/35#Interfaces.2FAPIs.2FDOM