Closed Bug 1431964 Opened 7 years ago Closed 7 years ago

Remove nsIDOMAttr

Categories

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

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

Attachments

(10 files)

(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
(deleted), patch
mccr8
: review+
Details | Diff | Splinter Review
There's no good reason for this to exist.
Depends on: 1431965
MozReview-Commit-ID: 5VTrCvHmMWi
Attachment #8944273 - Flags: review?(continuation)
MozReview-Commit-ID: G4HfD7ni9hd
Attachment #8944274 - Flags: review?(continuation)
MozReview-Commit-ID: 8eXx6IdmZ4W
Attachment #8944275 - Flags: review?(continuation)
MozReview-Commit-ID: 7l8CcT1ntL7
Attachment #8944276 - Flags: review?(continuation)
MozReview-Commit-ID: GnrOUhx9nTQ
Attachment #8944277 - Flags: review?(continuation)
MozReview-Commit-ID: 9GJmiVSI0bs
Attachment #8944278 - Flags: review?(continuation)
MozReview-Commit-ID: H8gyGnbqYtf
Attachment #8944279 - Flags: review?(continuation)
MozReview-Commit-ID: C7z0hcjC0Tg
Attachment #8944280 - Flags: review?(continuation)
MozReview-Commit-ID: 59TspEgvNRF
Attachment #8944281 - Flags: review?(continuation)
Attached patch part 10. Remove nsIDOMAttr (deleted) — Splinter Review
MozReview-Commit-ID: xj4QeXBF9V
Attachment #8944282 - Flags: review?(continuation)
Attachment #8944273 - Flags: review?(continuation) → review+
Attachment #8944274 - Flags: review?(continuation) → review+
Attachment #8944275 - Flags: review?(continuation) → review+
Thanks for splitting this up into small patches.
Blocks: 1132934
Attachment #8944276 - Flags: review?(continuation) → review+
Comment on attachment 8944277 [details] [diff] [review] part 5. Remove nsIDOMMozNamedAttrMap::Item Review of attachment 8944277 [details] [diff] [review]: ----------------------------------------------------------------- ::: editor/libeditor/HTMLURIRefObject.cpp @@ +91,5 @@ > NS_ENSURE_SUCCESS(rv, rv); > > // Loop over attribute list: > if (!mAttributes) { > + nsCOMPtr<dom::Element> element (do_QueryInterface(mNode)); nit: while you are here, maybe get rid of the space between element and (. ::: layout/inspector/inDOMView.cpp @@ +1245,5 @@ > > return NS_OK; > } > > nsresult You should change the return type of this method to void. (It only returns NS_OK, and it is only called by inDOMView::GetChildNodesFor, which does not check the return value.) ::: layout/xul/nsBox.cpp @@ +66,5 @@ > > nsIContent* content = GetContent(); > > // add on all the set attributes > + if (content && content->IsElement()) { I guess this would have crashed with a null deref in the old code if a non-element ended up in here? (on namedMap->GetLength())
Attachment #8944277 - Flags: review?(continuation) → review+
Attachment #8944278 - Flags: review?(continuation) → review+
Attachment #8944279 - Flags: review?(continuation) → review+
Attachment #8944280 - Flags: review?(continuation) → review+
Attachment #8944281 - Flags: review?(continuation) → review+
Attachment #8944282 - Flags: review?(continuation) → review+
> nit: while you are here, maybe get rid of the space between element and (. Done. > You should change the return type of this method to void. Done. > I guess this would have crashed with a null deref in the old code if a non-element ended up in here? Well, the old code doesn't even compile, since there is no GetAttributes on nsIDOMNode. All this stuff in nsBox.cpp is inside #ifdef DEBUG_LAYOUT which we apparently never really define or something...
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/22a83c38a0d7 part 1. Remove the XPCOM versions of GetAttributeNode(NS). r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/aac9cf730d20 part 2. Remove the XPCOM versions of createAttribute(NS). r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/8149c85354f0 part 3. Remove nsIDOMMozNamedAttrMap::GetNamedItem. r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/f42ff3c796bf part 4. Remove nsIDOMMozNamedAttrMap::GetNamedItemNS. r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/3d7a4d867c95 part 5. Remove nsIDOMMozNamedAttrMap::Item. r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/c8a113c0c9c2 part 6. Remove nsIDOMMozNamedAttrMap::GetLength. r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/be8aacd49efe part 7. Remove unused nsIDOMMozNamedAttrMap methods. r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/49ae9160211d part 8. Remove nsIDOMMozNamedAttrMap. r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/c93509796edc part 9. Stop using nsIDOMAttr in JS. r=mccr8 https://hg.mozilla.org/integration/mozilla-inbound/rev/cbea2cff1cc2 part 10. Remove nsIDOMAttr. r=mccr8
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: