Closed
Bug 1431964
Opened 7 years ago
Closed 7 years ago
Remove nsIDOMAttr
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
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.
Assignee | ||
Comment 1•7 years ago
|
||
MozReview-Commit-ID: 5VTrCvHmMWi
Attachment #8944273 -
Flags: review?(continuation)
Assignee | ||
Comment 2•7 years ago
|
||
MozReview-Commit-ID: G4HfD7ni9hd
Attachment #8944274 -
Flags: review?(continuation)
Assignee | ||
Comment 3•7 years ago
|
||
MozReview-Commit-ID: 8eXx6IdmZ4W
Attachment #8944275 -
Flags: review?(continuation)
Assignee | ||
Comment 4•7 years ago
|
||
MozReview-Commit-ID: 7l8CcT1ntL7
Attachment #8944276 -
Flags: review?(continuation)
Assignee | ||
Comment 5•7 years ago
|
||
MozReview-Commit-ID: GnrOUhx9nTQ
Attachment #8944277 -
Flags: review?(continuation)
Assignee | ||
Comment 6•7 years ago
|
||
MozReview-Commit-ID: 9GJmiVSI0bs
Attachment #8944278 -
Flags: review?(continuation)
Assignee | ||
Comment 7•7 years ago
|
||
MozReview-Commit-ID: H8gyGnbqYtf
Attachment #8944279 -
Flags: review?(continuation)
Assignee | ||
Comment 8•7 years ago
|
||
MozReview-Commit-ID: C7z0hcjC0Tg
Attachment #8944280 -
Flags: review?(continuation)
Assignee | ||
Comment 9•7 years ago
|
||
MozReview-Commit-ID: 59TspEgvNRF
Attachment #8944281 -
Flags: review?(continuation)
Assignee | ||
Comment 10•7 years ago
|
||
MozReview-Commit-ID: xj4QeXBF9V
Attachment #8944282 -
Flags: review?(continuation)
Updated•7 years ago
|
Attachment #8944273 -
Flags: review?(continuation) → review+
Updated•7 years ago
|
Attachment #8944274 -
Flags: review?(continuation) → review+
Updated•7 years ago
|
Attachment #8944275 -
Flags: review?(continuation) → review+
Comment 11•7 years ago
|
||
Thanks for splitting this up into small patches.
Updated•7 years ago
|
Attachment #8944276 -
Flags: review?(continuation) → review+
Comment 12•7 years ago
|
||
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+
Updated•7 years ago
|
Attachment #8944278 -
Flags: review?(continuation) → review+
Updated•7 years ago
|
Attachment #8944279 -
Flags: review?(continuation) → review+
Updated•7 years ago
|
Attachment #8944280 -
Flags: review?(continuation) → review+
Updated•7 years ago
|
Attachment #8944281 -
Flags: review?(continuation) → review+
Updated•7 years ago
|
Attachment #8944282 -
Flags: review?(continuation) → review+
Assignee | ||
Comment 13•7 years ago
|
||
> 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...
Comment 14•7 years ago
|
||
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
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/22a83c38a0d7
https://hg.mozilla.org/mozilla-central/rev/aac9cf730d20
https://hg.mozilla.org/mozilla-central/rev/8149c85354f0
https://hg.mozilla.org/mozilla-central/rev/f42ff3c796bf
https://hg.mozilla.org/mozilla-central/rev/3d7a4d867c95
https://hg.mozilla.org/mozilla-central/rev/c8a113c0c9c2
https://hg.mozilla.org/mozilla-central/rev/be8aacd49efe
https://hg.mozilla.org/mozilla-central/rev/49ae9160211d
https://hg.mozilla.org/mozilla-central/rev/c93509796edc
https://hg.mozilla.org/mozilla-central/rev/cbea2cff1cc2
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•