1320815 - deCOMtaminate nsIFrame::IsSelectable

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

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!

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

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 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: Bug 1320815 - Make nsIFrame::IsSelectable non-virtual, and move implementation from nsFrame to nsIFrame.

Review commit: https://reviewboard.mozilla.org/r/96092/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96092/

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: Bug 1320815 - DeCOMtaminate nsIFrame::IsSelectable by returning boolean instead of nsresult.

Review commit: https://reviewboard.mozilla.org/r/96094/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96094/

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

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

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8815106 [details]
Bug 1320815 - DeCOMtaminate nsIFrame::IsSelectable by returning boolean instead of nsresult.

https://reviewboard.mozilla.org/r/96094/#review96246

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/02cc5b7bf39c
https://hg.mozilla.org/mozilla-central/rev/ae6227686e47