nsAboutProtocolHandler::StartClone needs to change signature to continue overriding inherited method

RESOLVED FIXED in mozilla6

Status

()

Core
Networking
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla6
Points:
---

Firefox Tracking Flags

(firefox6-)

Details

(Whiteboard: [build_warning] bz nominated at comment 3, URL)

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
Bug 308590 changed nsSimpleURI::StartClone and its subclass's version nsSimpleNestedURI::StartClone to take one argument.

I missed that nsSimpleNestedURI *itself* had a subclass with a version that needed changing, too.  As a result, we currently get this build warning:
{
  nsSimpleNestedURI.h:80:26: warning: ‘virtual nsSimpleURI* nsSimpleNestedURI::StartClone(nsSimpleURI::RefHandlingEnum)’ was hidden
  nsAboutProtocolHandler.h:97:26: warning:   by ‘virtual nsSimpleURI* nsNestedAboutURI::StartClone()’
}

nsNestedAboutURI::StartClone needs to change to take an argument in order to continue overriding properly.

(I verified that this is the only instance of this type of bug by looking for "::Clone" and "::StartClone" in a build log)
(Assignee)

Comment 1

6 years ago
Created attachment 534605 [details] [diff] [review]
fix

Ultimately, it'd be nice for nsSimpleNestedURI::StartClone to replace its "new nsSimpleNestedURI()" call with some virtual function that nsNestedAboutURI can override (returning a "new nsNestedAboutURI" instead).

Then, we'll be able to get rid of nsNestedAboutURI's StartClone specialization.

However, for now, this is a simpler change.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #534605 - Flags: review?(bzbarsky)
(Assignee)

Updated

6 years ago
Summary: nsAboutProtocolHandler::StartClone needs to change signature to continue overriding parent → nsAboutProtocolHandler::StartClone needs to change signature to continue overriding inherited method
Comment on attachment 534605 [details] [diff] [review]
fix

r=me
Attachment #534605 - Flags: review?(bzbarsky) → review+
This needs to land in Fx6.
tracking-firefox6: --- → ?
(Assignee)

Comment 4

6 years ago
Created attachment 534688 [details] [diff] [review]
hg changeset for checkin
http://hg.mozilla.org/mozilla-central/rev/bb5904c365a2
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6

Updated

6 years ago
Whiteboard: [build_warning] → [build_warning] bz nominated at comment 3
I agree this needs to land, but doesn't really need to live on the tracking radar - a=me for aurora landing.
tracking-firefox6: ? → -
Attachment #534688 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 7

6 years ago
Comment on attachment 534605 [details] [diff] [review]
fix

Requesting approval to land on aurora, per comment 3.

Pretty trivial, low-risk fix - as described in comment 0, this just adds an argument to a method on nsNestedAboutURI, mirroring bug 308590's changes to the same method in nsNestedAboutURI's parent class.  This is necessary for the method to override correctly.
Attachment #534605 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 8

6 years ago
Comment on attachment 534605 [details] [diff] [review]
fix

oops, johnath beat me to the punch, a+'ing the newer revision of the patch. :) Canceling duplicate approval request.
Attachment #534605 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 9

6 years ago
Comment on attachment 534688 [details] [diff] [review]
hg changeset for checkin

gah, sorry, this is already in Aurora -- it landed on m-c a few hours before the branch point.

Canceling approval+, for clarity.
Attachment #534688 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.