The abstract method EditClient::didApplyStyle() does not have any parameters, WebEditorClient::didApplyStyle() should have the same signature.
Created attachment 255815 [details] Patch
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?
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!
Comment on attachment 255815 [details] Patch Clearing flags on attachment: 255815 Committed r186114: <http://trac.webkit.org/changeset/186114>
All reviewed patches have been landed. Closing bug.
(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.