Bug 146450 – [WinCairo] Compile error, WebEditorClient::didApplyStyle() should not have any parameters.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

The abstract method EditClient::didApplyStyle() does not have any parameters, WebEditorClient::didApplyStyle() should have the same signature.

Created attachment 255815 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 255815 [details]
Patch

LGTM, r=me. Could you add an override keyword before 
landing to avoid possible bugs in the future here?

( Just note: This code was introduced in bug146379 - https://trac.webkit.org/changeset/186086 )

(In reply to comment #2)
> Comment on attachment 255815 [details]
> Patch
> 
> LGTM, r=me. Could you add an override keyword before 
> landing to avoid possible bugs in the future here?
> 
> ( Just note: This code was introduced in bug146379 -
> https://trac.webkit.org/changeset/186086 )

Thanks for reviewing :)

Yes, I can add the override keyword, but I seem to remember there was an agreement not to add the override keyword when overriding pure virtual methods?

[Csaba Osztrogonác]

Comment on attachment 255815 [details]
Patch

I missed this agreement. But I agree, adding override keyword for 
overriding pure virtual function isn't necessary. Let's land it as is.

(In reply to comment #4)
> Comment on attachment 255815 [details]
> Patch
> 
> I missed this agreement. But I agree, adding override keyword for 
> overriding pure virtual function isn't necessary. Let's land it as is.

Ok, thanks!

[WebKit Commit Bot]

Comment on attachment 255815 [details]
Patch

Clearing flags on attachment: 255815

Committed r186114: <http://trac.webkit.org/changeset/186114>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

(In reply to comment #3)
> I seem to remember there was an
> agreement not to add the override keyword when overriding pure virtual
> methods?

This is the first I have heard of this. I think we should definitely use override in cases like that and I’d like to know more about why we wouldn’t.

(In reply to comment #8)
> (In reply to comment #3)
> > I seem to remember there was an
> > agreement not to add the override keyword when overriding pure virtual
> > methods?
> 
> This is the first I have heard of this. I think we should definitely use
> override in cases like that and I’d like to know more about why we wouldn’t.

Ok, sorry for spreading false rumours. I thought I picked this up somewhere, but I am probably wrong.