ImageDocument::mImageContent should probably be an Element rather than an nsIContent

RESOLVED FIXED in Firefox 48

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Ms2ger, Assigned: sleroux, Mentored)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [lang=c++][good first bug] btpp-backlog)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
This would allow us to remove the AsElement() calls.

Code: dom/html/ImageDocument.h, dom/html/ImageDocument.cpp
Whiteboard: [lang=c++][good first bug] → [lang=c++][good first bug] btpp-backlog
(Assignee)

Comment 1

3 years ago
I wouldn't mind taking this to try out something I learned from the onboarding session. Is the ask to just change the nsCOMPtr to use Element type instead of nsIContent to avoid the AsElement calls?
(Reporter)

Comment 2

3 years ago
Yes, that's correct
(Assignee)

Updated

3 years ago
Assignee: nobody → sleroux
Status: NEW → ASSIGNED
(Assignee)

Comment 3

3 years ago
Created attachment 8726382 [details]
MozReview Request: Bug 1252015 - Changed mImageContent reference to be Element type instead of nsIContent; r?baku

Review commit: https://reviewboard.mozilla.org/r/37947/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37947/
Attachment #8726382 - Flags: review?(Ms2ger)
(Reporter)

Updated

3 years ago
Attachment #8726382 - Flags: review?(Ms2ger) → review?(amarchesini)
Comment on attachment 8726382 [details]
MozReview Request: Bug 1252015 - Changed mImageContent reference to be Element type instead of nsIContent; r?baku

https://reviewboard.mozilla.org/r/37947/#review34711
Attachment #8726382 - Flags: review?(amarchesini) → review+
(Reporter)

Comment 5

3 years ago
Thanks!

Sheriff: please update the r= in the commit message, thanks!
Keywords: checkin-needed
has problems to apply:

applying 647c12191f36
patching file dom/html/ImageDocument.cpp
Hunk #1 FAILED at 334
Hunk #3 FAILED at 486
2 out of 4 hunks FAILED -- saving rejects to file dom/html/ImageDocument.cpp.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue
Flags: needinfo?(sleroux)

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Comment 7

3 years ago
I've gone ahead and transplanted from latest. Do I push it back up to reviewboard?
Flags: needinfo?(sleroux)
(Reporter)

Comment 8

3 years ago
I think so, yes/
(Assignee)

Comment 9

3 years ago
Comment on attachment 8726382 [details]
MozReview Request: Bug 1252015 - Changed mImageContent reference to be Element type instead of nsIContent; r?baku

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37947/diff/1-2/
Attachment #8726382 - Attachment description: MozReview Request: Bug 1252015 - Changed mImageContent reference to be Element type instead of nsIContent; r?Ms2ger → MozReview Request: Bug 1252015 - Changed mImageContent reference to be Element type instead of nsIContent; r?baku
(Reporter)

Updated

3 years ago
Keywords: checkin-needed

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fd6bbe2b188e
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.