Closed
Bug 966240
Opened 10 years ago
Closed 6 years ago
Drop support for MSThemeCompatible
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla56
People
(Reporter: fb+mozdev, Assigned: MatsPalmgren_bugz)
References
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(1 file)
3.47 KB,
patch
|
dbaron
:
review+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
meta[http-equiv="MSThemeCompatible"] is non-standard and produces bugs.
Reporter | ||
Updated•10 years ago
|
![]() |
||
Updated•7 years ago
|
Component: Widget → DOM
Updated•6 years ago
|
Keywords: dev-doc-needed,
site-compat
Assignee | ||
Comment 3•6 years ago
|
||
It seems that neither Chrome, Safari or Edge support this feature.
Assignee: nobody → mats
Assignee | ||
Comment 4•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f92d296c60f7aa2acbff9479f6a0b3d603a24c60
Assignee | ||
Comment 5•6 years ago
|
||
Attachment #8878271 -
Flags: review?(dbaron)
Comment 6•6 years ago
|
||
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•6 years ago
|
||
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
Attachment #8878271 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 8•6 years ago
|
||
Good point, I removed the PresShell bits too.
Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/47143d875634 Remove support for <meta http-equiv="msthemecompatible" content="no">. r=dbaron
Comment 10•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/47143d875634
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee | ||
Comment 11•6 years ago
|
||
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
Attachment #8878271 -
Flags: approval-mozilla-beta?
Comment 12•6 years ago
|
||
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•6 years ago
|
||
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
Attachment #8878271 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 14•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/4db974b6413e
status-firefox55:
--- → fixed
Updated•6 years ago
|
Comment 16•6 years ago
|
||
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.
Flags: needinfo?(mats)
Comment 18•6 years ago
|
||
(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?
Flags: needinfo?(mats)
Comment 19•6 years ago
|
||
(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.
Assignee | ||
Comment 20•6 years ago
|
||
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.
Flags: needinfo?(mats)
Comment 21•6 years ago
|
||
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.
Keywords: dev-doc-needed → dev-doc-complete
Updated•4 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•