Closed Bug 1055773 Opened 9 years ago Closed 9 years ago

Move hasAttributes() to Element


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

Not set





(Reporter: annevk, Assigned: nl, Mentored)


(Keywords: dev-doc-complete)


(1 file, 4 obsolete files)

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

Prior: bug 849661 and bug 856752.
Blocks: 1055776
"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)."
No longer blocks: 1055776
Summary: Move hasAttributes() and attributes to Element → Move hasAttributes() to Element
Mentor: bzbarsky
Assignee: nobody → nicklebedev37
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 : and second after i did it:
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.
Flags: needinfo?(Ms2ger)
Updated uuids according to bz's comment.
Attachment #8489255 - Attachment is obsolete: true
Attachment #8489444 - Flags: review+
Keywords: checkin-needed
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!
Keywords: checkin-needed
Oops, sorry. Rebased.
Attachment #8489444 - Attachment is obsolete: true
Attachment #8489862 - Flags: review+
Keywords: checkin-needed

And a follow-up for other UUID bumps that |mach update-uuids nsIDOMElement nsIDOMNode| say need updating:
Keywords: checkin-needed
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.
Flags: needinfo?(Ms2ger)
Could you please clarify how I should update the following files:

1. Here I need simply remove the following line,
Am I correct?

2. Should I simply remove the following line too? Or change it to PASS?

3. How would you advice to fix the failure on this line:
Flags: needinfo?(Ms2ger)
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.
Flags: needinfo?(Ms2ger)
Attached patch has_attr (obsolete) — Splinter Review
Try results for all mochi/webplatform tests: It contains one small issue which i've fixed and it is reflected in the second try results:
Attachment #8489862 - Attachment is obsolete: true
Attachment #8498150 - Flags: review?(bzbarsky)
Comment on attachment 8498150 [details] [diff] [review]

>+++ 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+
Attachment #8498150 - Attachment is obsolete: true
Attachment #8498289 - Flags: review+
Keywords: checkin-needed
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.