Closed Bug 1320815 Opened 3 years ago Closed 3 years ago

deCOMtaminate nsIFrame::IsSelectable

Categories

(Core :: Selection, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: dbaron, Assigned: dbaron)

Details

Attachments

(2 files)

While reviewing bug 389283, I noticed:

 * nsIFrame::IsSelectable is virtual, but is implemented only on nsFrame and not overridden by subclasses.  It should just be non-virtual, with the implementation on nsIFrame instead of nsFrame.

 * it returns nsresult and has a boolean out parameter, but the only time it returns a failure result is if the caller provides a null out-parameter.  It should just return the boolean result instead!
I wonder if there is any tool which can scan the whole build and list all virtual functions which only have one implementation (which means they probably shouldn't be virtual).
Comment on attachment 8815105 [details]
Bug 1320815 - Make nsIFrame::IsSelectable non-virtual, and move implementation from nsFrame to nsIFrame.

https://reviewboard.mozilla.org/r/96092/#review96244
Attachment #8815105 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8815106 [details]
Bug 1320815 - DeCOMtaminate nsIFrame::IsSelectable by returning boolean instead of nsresult.

https://reviewboard.mozilla.org/r/96094/#review96246
Attachment #8815106 - Flags: review?(xidorn+moz) → review+
Pushed by dbaron@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/02cc5b7bf39c
Make nsIFrame::IsSelectable non-virtual, and move implementation from nsFrame to nsIFrame.  r=xidorn
https://hg.mozilla.org/integration/autoland/rev/ae6227686e47
DeCOMtaminate nsIFrame::IsSelectable by returning boolean instead of nsresult.  r=xidorn
https://hg.mozilla.org/mozilla-central/rev/02cc5b7bf39c
https://hg.mozilla.org/mozilla-central/rev/ae6227686e47
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.