966240 - Drop support for MSThemeCompatible

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Florian Bender]

meta[http-equiv="MSThemeCompatible"] is non-standard and produces bugs.

Comment 3

[Mats Palmgren (inactive)]

It seems that neither Chrome, Safari or Edge support this feature.

Comment 4

[Mats Palmgren (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f92d296c60f7aa2acbff9479f6a0b3d603a24c60

Comment 5

[Mats Palmgren (inactive)]

Attachment: Remove support for <meta http-equiv="msthemecompatible" content="no">

Comment 6

[Masatoshi Kimura [:emk]]

Comment on attachment 8878271 [details] [diff] [review]
Remove support for <meta http-equiv="msthemecompatible" content="no">

Review of attachment 8878271 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsContentSink.cpp
@@ -345,5 @@
> -    nsAutoString value(aValue);
> -    if (value.LowerCaseEqualsLiteral("no")) {
> -      nsIPresShell* shell = mDocument->GetShell();
> -      if (shell) {
> -        shell->DisableThemeSupport();

Do we still need nsIPresShell::DisableThemeSupport? Apparently this is the only caller [1] and nsIPresShell is not scriptable.

We could also remove nsGkAtoms::msthemecompatible.

[1] https://dxr.mozilla.org/mozilla-central/search?q=DisableThemeSupport

Comment 7

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

Comment on attachment 8878271 [details] [diff] [review]
Remove support for <meta http-equiv="msthemecompatible" content="no">

Please also remove nsIPresShell::mIsThemeSupportDisabled.

r=dbaron with that

Comment 8

[Mats Palmgren (inactive)]

Good point, I removed the PresShell bits too.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47143d875634
Remove support for <meta http-equiv="msthemecompatible" content="no">.  r=dbaron

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/47143d875634

Comment 11

[Mats Palmgren (inactive)]

Comment on attachment 8878271 [details] [diff] [review]
Remove support for <meta http-equiv="msthemecompatible" content="no">

Approval Request Comment
[Feature/Bug causing the regression]: The culprit is the "msthemecompatible" feature as such, but it was mostly harmless until bug 605985 landed which made checkbox/radio controls invisible when "msthemecompatible" is requested
[User impact if declined]: invisible checkbox/radio controls (I suspect that "msthemecompatible" is very rare on the web though)
[Is this code covered by automated tests?]:no
[Has the fix been verified in Nightly?]:not yet
[Needs manual test from QE? If yes, steps to reproduce]: sure, there's a testcase in bug 1373417
[List of other uplifts needed for the feature/fix]:none
[Is the change risky?]:no
[Why is the change risky/not risky?]:it removes a feature that none of the other major browsers support, and the patch is fairly trivial
[String changes made/needed]:none

Comment 12

[Kohei Yoshino [:kohei]]

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/checkboxes-and-radio-buttons-are-not-displayed-when-msthemecompatible-is-disabled/

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 8878271 [details] [diff] [review]
Remove support for <meta http-equiv="msthemecompatible" content="no">

sounds sensible enough, beta55+

Should be in 55.0b3

Comment 14

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-beta/rev/4db974b6413e

Comment 15

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Already verified in bug 1373417.

Comment 16

[Kohei Yoshino [:kohei]]

As mentioned in Bug 1373417, the patch requires an uplift request for the release channel so the bug will be fixed with 54.0.1.

Comment 17

[Mats Palmgren (inactive)]

Not my decision to make.

Comment 18

[Masatoshi Kimura [:emk]]

(In reply to Mats Palmgren (:mats) from comment #17)
> Not my decision to make.

Then who is making the decision? I thought developers (you in this case) make an uplift request and release drivers make a decision. No?

Comment 19

[Kohei Yoshino [:kohei]]

(In reply to Masatoshi Kimura [:emk] from comment #18)
> Then who is making the decision? I thought developers (you in this case)
> make an uplift request and release drivers make a decision. No?

Believe so.

Comment 20

[Mats Palmgren (inactive)]

I generally don't request uplift to the _release channel_ unless it's for
a major web-compat or crash or security issue.  This bug doesn't fit that
criteria IMO.

Comment 21

[Chris Mills [:cmills]]

I've added a note to the Fx55 rel notes to cover this:

https://developer.mozilla.org/en-US/Firefox/Releases/55#HTML_2

I didn't make any changes to the ref docs, as we never mentioned this value in the first place.