Move hasAttributes() to Element

RESOLVED FIXED in mozilla35

Status

()

defect
RESOLVED FIXED
5 years ago
3 months ago

People

(Reporter: annevk, Assigned: nl, Mentored)

Tracking

({dev-doc-complete})

unspecified
mozilla35
x86
macOS
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Reporter

Description

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

Prior: bug 849661 and bug 856752.
Reporter

Updated

5 years ago
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)."

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

Updated

5 years ago
No longer blocks: 1055776
Summary: Move hasAttributes() and attributes to Element → Move hasAttributes() to Element
Mentor: bzbarsky
Assignee

Updated

5 years ago
Assignee: nobody → nicklebedev37
Assignee

Comment 2

5 years ago
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)
Assignee

Comment 3

5 years ago
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.
Flags: needinfo?(Ms2ger)
Assignee

Comment 6

5 years ago
Updated uuids according to bz's comment.
Attachment #8489255 - Attachment is obsolete: true
Attachment #8489444 - Flags: review+
Assignee

Updated

5 years ago
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
Assignee

Comment 8

5 years ago
Oops, sorry. Rebased.
Attachment #8489444 - Attachment is obsolete: true
Attachment #8489862 - Flags: review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
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
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)
Assignee

Comment 12

5 years ago
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
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)
Assignee

Comment 14

5 years ago
Posted patch has_attr (obsolete) — Splinter Review
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.
Attachment #8489862 - Attachment is obsolete: true
Attachment #8498150 - Flags: review?(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.
Attachment #8498150 - Flags: review?(bzbarsky) → review+
Assignee

Comment 16

5 years ago
Done.
Attachment #8498150 - Attachment is obsolete: true
Attachment #8498289 - Flags: review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c3fc41feeb22
Status: NEW → RESOLVED
Closed: 5 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.