Closed Bug 1055773 Opened 9 years ago Closed 9 years ago
Attributes() to Element
It seems Chrome successfully moved these from node to element. So should we. Prior: bug 849661 and bug 856752.
"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
No longer blocks: 1055776
Summary: Move hasAttributes() and attributes to Element → Move hasAttributes() 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.
Attachment #8489255 - Flags: review?(bzbarsky)
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 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.
Attachment #8489255 - Flags: review?(bzbarsky) → review+
ms2ger, note the imptests change here.
Updated uuids according to bz's comment.
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!
Oops, sorry. Rebased.
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
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
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.
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
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.
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 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.
Attachment #8498150 - Flags: review?(bzbarsky) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.