Closed
Bug 672190
Opened 13 years ago
Closed 12 years ago
consider removing expandEntityReferences from NodeIterator and TreeWalker
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: annevk, Assigned: gaurav.pruthi88)
Details
(Keywords: addon-compat, dev-doc-complete, Whiteboard: [good first bug][mentor=Ms2ger][lang=c++])
Attachments
(1 file, 4 obsolete files)
3.59 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
This attribute has never made much sense.
Reporter | ||
Updated•13 years ago
|
Summary: consider removing expandEntityReferences from NodeIterator → consider removing expandEntityReferences from NodeIterator and TreeWalker
Comment 1•13 years ago
|
||
This is done in Bug 698384.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 2•12 years ago
|
||
The actual attribute wasn't removed in that bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 3•12 years ago
|
||
http://mxr.mozilla.org/mozilla-central/search?string=expandEntityReferences shows it's still alive. This was about the attribute on the interfaces.
Updated•12 years ago
|
Status: REOPENED → NEW
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [good first bug][mentor=Ms2ger][lang=c++]
Version: unspecified → Trunk
Assignee | ||
Comment 4•12 years ago
|
||
Hi, Can you please assign this bug to me.
Updated•12 years ago
|
Assignee: nobody → gaurav.pruthi88
Assignee | ||
Comment 5•12 years ago
|
||
Please find attach the diff file for the changes made in files nsIDOMTreeWalker.idl and nsIDOMNodeIterator.idl.I have made the changes , tested and build the mozilla-central after pushing the changes to my local rep.
pls chk...its my first :)
Attachment #699038 -
Flags: review+
Comment 6•12 years ago
|
||
Comment on attachment 699038 [details] [diff] [review]
Removed the field "expandEntityReferences" from nsIDOMTreeWalker.idl and nsIDOMNodeIterator.idl
First, looks like you changed the files you touched to use windows line endings (CR+LF); we use unix line endings (LF). IIRC, Mozillabuild includes a dos2unix script that can fix that.
Second, you'll need to remove the implementation of those attributes too; these are at <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsNodeIterator.cpp#213> and <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsTreeWalker.cpp#90>.
Could you fix those issues and request review from me?
Attachment #699038 -
Attachment is patch: true
Attachment #699038 -
Flags: review+
Assignee | ||
Comment 7•12 years ago
|
||
Uploading the new patch...pls check again.
Attachment #699038 -
Attachment is obsolete: true
Attachment #699130 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Attachment #699130 -
Flags: review+ → review?(Ms2ger)
Comment 8•12 years ago
|
||
Comment on attachment 699130 [details] [diff] [review]
Modified patch including the expandEntityReferences attribute removed from nsIDOMNodeIterator.idl and nsIDOMTreeWalker.idl as well as getter method removed from nsTreeWalker.cpp and nsNodeIterator.cpp
Review of attachment 699130 [details] [diff] [review]:
-----------------------------------------------------------------
Looks like I forgot an implementation at <http://mxr.mozilla.org/mozilla-central/source/layout/inspector/src/inDeepTreeWalker.cpp#109>, which you'll need to remove as well; sorry about that.
It seems you still need to fix the windows line endings; looks good otherwise!
::: dom/interfaces/traversal/nsIDOMNodeIterator.idl
@@ +9,5 @@
> +interface nsIDOMNodeIterator;
> +interface nsIDOMNodeFilter;
> +
> +
> +[scriptable, uuid(5af83f50-c8d5-4824-be29-1aa9d640bacb)]
You'll need to update this uuid, for example with one from <http://mozilla.pettay.fi/cgi-bin/mozuuid.pl>.
::: dom/interfaces/traversal/nsIDOMTreeWalker.idl
@@ +9,5 @@
> +interface nsIDOMTreeWalker;
> +interface nsIDOMNodeFilter;
> +
> +
> +[scriptable, uuid(400af3ca-1dd2-11b2-a50a-887ecca2e63a)]
And this one, too
Attachment #699130 -
Flags: review?(Ms2ger) → feedback+
Assignee | ||
Comment 9•12 years ago
|
||
Removed the implementation at /mozilla-central/source/layout/inspector/src/inDeepTreeWalker.cpp and I've inserted the uuid for each change after re-running the script mentioned @http://mozilla.pettay.fi/cgi-bin/mozuuid.pl .
pls check
Attachment #699130 -
Attachment is obsolete: true
Attachment #699624 -
Flags: review?(Ms2ger)
Comment 10•12 years ago
|
||
Gaurav, the patch looks good, except for the line ending issue. Do you need help fixing that?
Assignee | ||
Comment 11•12 years ago
|
||
I have used dos2unix tool also, in Vi ':set ff' cmd on file shows fileformat=unix, Do tell if there is any other solution.
Comment 12•12 years ago
|
||
Comment on attachment 699624 [details] [diff] [review]
Updated diff file with removed getter method for expandEntityReferences from '/mozilla-central/source/layout/inspector/src/inDeepTreeWalker.cpp' in continuation with earlier attached patch.
I'll fix the CRs on landing.
Attachment #699624 -
Flags: review?(bugs)
Attachment #699624 -
Flags: review?(Ms2ger)
Attachment #699624 -
Flags: feedback+
Comment 13•12 years ago
|
||
Comment on attachment 699624 [details] [diff] [review]
Updated diff file with removed getter method for expandEntityReferences from '/mozilla-central/source/layout/inspector/src/inDeepTreeWalker.cpp' in continuation with earlier attached patch.
Would be great to have this in correct format (unix line endings and only one commit message), but if Ms2ger can fix that before landing...
This is a tiny bit regression risky since some silly web page
may use expandEntityReferences property, even though it returns always false.
But we're quite early in the cycle so we can back this out if it is causes
problems.
Attachment #699624 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 14•12 years ago
|
||
Adding the new patch. I build mozilla on linux and have used MQ for this patch, hope line endings in this one will be corrected.
Attachment #699624 -
Attachment is obsolete: true
Attachment #703967 -
Flags: review?(Ms2ger)
Comment 15•12 years ago
|
||
Comment on attachment 703967 [details] [diff] [review]
Uploading the new patch with unix endings using MQ.
Review of attachment 703967 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks! Looks like you forgot to update the uuids in this patch, though.
Assignee | ||
Comment 16•12 years ago
|
||
Attachment #703967 -
Attachment is obsolete: true
Attachment #703967 -
Flags: review?(Ms2ger)
Attachment #704318 -
Flags: review?(Ms2ger)
Comment 17•12 years ago
|
||
Comment on attachment 704318 [details] [diff] [review]
Uploading the new patch with updated 'uuid' in IDL files.
Thanks!
Attachment #704318 -
Flags: review?(Ms2ger) → review+
Updated•12 years ago
|
Keywords: checkin-needed
Comment 18•12 years ago
|
||
Keywords: checkin-needed
Updated•12 years ago
|
Keywords: addon-compat,
dev-doc-needed
Comment 19•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment 20•12 years ago
|
||
I've added this bug to the compatibility doc. Please correct the info if I'm wrong.
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_21
Updated•12 years ago
|
Component: DOM: Traversal-Range → DOM: Core & HTML
Comment 21•11 years ago
|
||
I've also updated:
https://developer.mozilla.org/en-US/docs/Web/API/TreeWalker.expandEntityReferences
https://developer.mozilla.org/en-US/docs/Web/API/NodeIterator.expandEntityReferences
and
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/21
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•