Bug 195597 – Remove MediaWiki site specific quirks

• Component: CSS
• Status: RESOLVED FIXED

[Ryosuke Niwa]

A site specific quirk was added to workaround KHTML workaround in MediaWiki in https://bugs.webkit.org/show_bug.cgi?id=28350
Chromium removed this workaround way back in 2013: https://github.com/chromium/chromium/commit/ecf84fc9c1a51c8ede7adfd0b0cba446d9a8caa0

It's 2019 now. Six years should be more than enough buffer for us to follow the suit.

[Ryosuke Niwa]

Created attachment 364341 [details]
cleanup

[Simon Fraser (smfr)]

Comment on attachment 364341 [details]
cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=364341&action=review

> Source/WebCore/css/parser/CSSParserContext.cpp:-82
> -    needsSiteSpecificQuirks = true;

Yay for removing this!

[Ryosuke Niwa]

Waiting for EWS...

[Daniel Bates]

Comment on attachment 364341 [details]
cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=364341&action=review

I got it. You want a pat on the back. Good job.

> Source/WebCore/ChangeLog:8
> +        In the afternoon of the bright sunny [1] seventeenth day of August in the year two thousand and nine,

Why are you writing like this? It’s not fun or funny in my eyes, but maybe it will make someone laugh (not counting yourself) :/ Also when I read it the first thought I had was Ryosuke is trying to be sarcastic, but as I continued reading I was liked, yeah doesn’t seem sarcastic maybe he’s trying to be funny. Didn’t laugh though.

> Source/WebCore/ChangeLog:11
> +        Blink since removed this workaround on the seventh day of November in the year two thousand thirteen:

Talk about beating a dead horse......

> Source/WebCore/ChangeLog:15
> +        workaround obsolete and unnecessary for the Web compatibility.

It’s a joke. Guess I am a tough crowd because I didn’t find this funny. :/

> Source/WebCore/ChangeLog:17
> +        [1] https://www.wunderground.com/history/daily/KNUQ/date/2009-8-17?reqdb.zip=95014

I don’t get it. Is this a joke?

> Source/WebCore/css/parser/CSSParserContext.cpp:-63
> -    needsSiteSpecificQuirks = document.settings().needsSiteSpecificQuirks();

No way! Sorry, I don’t believe you. As much as I want to get rid of this, your joke and the linked Blink commit don’t support seem to support the premise that you can remove this or maybe the joke just left a very bad taste in my mouth. No, it was both.

>> Source/WebCore/css/parser/CSSParserContext.cpp:-82
>> -    needsSiteSpecificQuirks = true;
> 
> Yay for removing this!

Ok, Simon is convinced. That’s good enough.

[Ryosuke Niwa]

Created attachment 364351 [details]
Patch for landing

[Ryosuke Niwa]

Comment on attachment 364351 [details]
Patch for landing

Wait for EWS.

[Daniel Bates]

Comment on attachment 364351 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=364351&action=review

> Source/WebCore/ChangeLog:15
> +        workaround obsolete and unnecessary for the Web compatibility.

Not grammatically correct. "the Web"?

[Daniel Bates]

Comment on attachment 364351 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=364351&action=review

> Source/WebCore/ChangeLog:9
> +        we added a site specific quirk for the KHTML workaround in MediaWiki in the WebKit revision 47383.

Not grammatically correct. "the WebKit"?

[Daniel Bates]

oh, this is going to be fun.

[Daniel Bates]

Comment on attachment 364351 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=364351&action=review

> Source/WebCore/ChangeLog:8
> +        In the afternoon of the bright sunny [1] seventeenth day of August in the year two thousand and nine,

of the? Doesn't sound correct.

[Ryosuke Niwa]

Dan, please stop making further comments on this bug. Your behavior is at least passive aggressive if not outright offensive.

[Daniel Bates]

(In reply to Ryosuke Niwa from comment #11)
> Dan, please stop making further comments on this bug. Your behavior is at
> least passive aggressive if not outright offensive.

I apologize if it came off this way. It wasn't my intention to do either. I just wanted to help ensure that the commit message was accurate because it doesn't correspond to the code change or explain why this code change is correct. For comparison, the Blink commit you referenced has an understandable commit message that is written in layman terminology that clearly identifies the reason for the code change. Your change goes above and beyond the Blink change and isn't very clear. I don't understand why we need to try to make jokes in the ChangeLog instead of just clearly stating the reason for the change and especially for the bits that aren't corresponding to the Blink change.

[Ryosuke Niwa]

Committed r242780: <https://trac.webkit.org/changeset/242780>

[Radar WebKit Bug Importer]

<rdar://problem/48798911>

[Daniel Bates]

(In reply to Ryosuke Niwa from comment #13)
> Committed r242780: <https://trac.webkit.org/changeset/242780>

Thank you, Ryosuke, for updating the ChangeLog. It reads well and clearly describes both the reason for the quirk and the reason you feel confident about removing it. It is in my opinion 1000x better than the original.

[Ryosuke Niwa]

(In reply to Daniel Bates from comment #12)
> (In reply to Ryosuke Niwa from comment #11)
> > Dan, please stop making further comments on this bug. Your behavior is at
> > least passive aggressive if not outright offensive.
> 
> I apologize if it came off this way. It wasn't my intention to do either. I
> just wanted to help ensure that the commit message was accurate because it
> doesn't correspond to the code change or explain why this code change is
> correct. For comparison, the Blink commit you referenced has an
> understandable commit message that is written in layman terminology that
> clearly identifies the reason for the code change. Your change goes above
> and beyond the Blink change and isn't very clear. I don't understand why we
> need to try to make jokes in the ChangeLog instead of just clearly stating
> the reason for the change and especially for the bits that aren't
> corresponding to the Blink change.

If that were your intention, please state from the get-go instead of keep making sneaky & passive aggressive comments in the future. FWIW, I received them as personal attacks.

[Daniel Bates]

(In reply to Ryosuke Niwa from comment #16)
> (In reply to Daniel Bates from comment #12)
> > (In reply to Ryosuke Niwa from comment #11)
> > > Dan, please stop making further comments on this bug. Your behavior is at
> > > least passive aggressive if not outright offensive.
> > 
> > I apologize if it came off this way. It wasn't my intention to do either. I
> > just wanted to help ensure that the commit message was accurate because it
> > doesn't correspond to the code change or explain why this code change is
> > correct. For comparison, the Blink commit you referenced has an
> > understandable commit message that is written in layman terminology that
> > clearly identifies the reason for the code change. Your change goes above
> > and beyond the Blink change and isn't very clear. I don't understand why we
> > need to try to make jokes in the ChangeLog instead of just clearly stating
> > the reason for the change and especially for the bits that aren't
> > corresponding to the Blink change.
> 
> If that were your intention, please state from the get-go instead of keep
> making sneaky & passive aggressive comments in the future. FWIW, I received
> them as personal attacks.

I will keep this in mind moving forward. Never meant for my remarks to be taken as an attack.