consider removing expandEntityReferences from NodeIterator and TreeWalker

RESOLVED FIXED in mozilla21

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: annevk, Assigned: Gaurav Pruthi)

Tracking

({addon-compat, dev-doc-complete})

Trunk
mozilla21
addon-compat, dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=Ms2ger][lang=c++])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

6 years ago
This attribute has never made much sense.
(Reporter)

Updated

6 years ago
Summary: consider removing expandEntityReferences from NodeIterator → consider removing expandEntityReferences from NodeIterator and TreeWalker
This is done in Bug 698384.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
The actual attribute wasn't removed in that bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 3

4 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

4 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

4 years ago
Hi, Can you please assign this bug to me.

Updated

4 years ago
Assignee: nobody → gaurav.pruthi88
(Assignee)

Comment 5

4 years ago
Created attachment 699038 [details] [diff] [review]
Removed the field "expandEntityReferences" from nsIDOMTreeWalker.idl and nsIDOMNodeIterator.idl

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 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

4 years ago
Created 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

Uploading the new patch...pls check again.
Attachment #699038 - Attachment is obsolete: true
Attachment #699130 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #699130 - Flags: review+ → review?(Ms2ger)
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

4 years ago
Created 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.

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)
Gaurav, the patch looks good, except for the line ending issue. Do you need help fixing that?
(Assignee)

Comment 11

4 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 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 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

4 years ago
Created attachment 703967 [details] [diff] [review]
Uploading the new patch with unix endings using MQ.

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 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

4 years ago
Created attachment 704318 [details] [diff] [review]
Uploading the new patch with updated 'uuid' in IDL files.
Attachment #703967 - Attachment is obsolete: true
Attachment #703967 - Flags: review?(Ms2ger)
Attachment #704318 - Flags: review?(Ms2ger)
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

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/56efd49e70dd
Keywords: checkin-needed

Updated

4 years ago
Keywords: addon-compat, dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/56efd49e70dd
Status: NEW → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
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
Component: DOM: Traversal-Range → DOM: Core & HTML
Product: Core → Core
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.