STR: 1. Visit a site whose domain is on the CSS Unprefixing Whitelist, e.g.: http://3g.163.com/touch/ 2. In the Web Console, set some element's style.display to "-webkit-box", e.g.: document.documentElement.style.display = "-webkit-box" 3. Open a new tab. ACTUAL RESULTS: Abort with: { Assertion failure: !mDidUnprefixWebkitBoxInEarlierDecl (Someone forgot to clear the 'did unprefix webkit-box' flag), at layout/style/nsCSSParser.cpp:6180 } This is due to a bug in http://hg.mozilla.org/mozilla-central/rev/57410350de4d (Bug 1107378 part 3). That patch assumed that all entry-points into "display"-parsing code would be surrounded with an AutoRestore, but it didn't cover the parsing entry-point that's exercised in this case -- nsCSSParser::ParseProperty.

So stepping back -- the intention of this flag (mDidUnprefixWebkitBoxInEarlierDecl) is to handle cases where we've got: .box { display: -webkit-box; display: -moz-box; } A naive unprefixing strategy would have no effect on style like the above, because the "moz-box" styling will stomp on our modern-flexbox unprefixed version of the "webkit-box". So, this flag is intended to track whether we unprefixed "display:-webkit-box" (to display:flex) earlier in a block of declarations -- and if we did, then we treat later "display: -moz-box" as "display:flex", too, rather than letting it stomp on our earlier unprefixed style. As this bug shows, we're setting this flag too eagerly right now -- in cases where we're clearly *not* parsing a block of declarations (and hence don't have to worry about upcoming "-moz-box" styling). We should be more restrictive about when we set this bit of state. I've got a local patch to change this bool into a 3-state enum, with the states being (1) not currently parsing a block of declarations (the ground state) (2) Parsing a block of declarations, and have not yet unprefixed a "display:-webkit-box" decl (3) Parsing a block of declarations, and *have* unprefixed a "display:-webkit-box" decl. State 3 is the active state where our behavior actually changes & we start unprefixing "display:-moz-box". With this 3-state setup, we'll now be entering this state with *actual* confidence that we've got an AutoRestore somewhere up the stack, which will reset us to state 1 when the block of declarations is complete.

(Still needs an automated test; working on one now)

Comment on attachment 8609488 [details] [diff] [review] fix v2 (now with test & commit message) >+ enum WebkitBoxUnprefixState { Maybe an explicit ": uint8_t" would be good, given that I'm not sure what size it will default to otherwise? (I somewhat suspect it will end up being int-sized otherwise.) r=dbaron

Good call; added that, & pushed to Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5240c50fcf66 Thanks for the review!

Comment on attachment 8609488 [details] [diff] [review] fix v2 (now with test & commit message) I'd like to backport this fix to Aurora & Beta. Approval Request Comment [Feature/regressing bug #]: The "CSS Unprefixing service", which lets us interpret -webkit prefixed CSS as approximately-equivalent styles that we support. This feature was implemented in bug 1165834, & enabled by default in bug 1143147. (So, this goes back to Firefox 39, which is currently Beta.) [User impact if declined]: Potential for broken layout, *if* a user visits any of the sites on our unprefixing whitelist, and that site happens to set anyElement.style.display to "-webkit-box" using JavaScript. If that happens, the user could have broken layout on other visited sites, for the rest of their browsing session, due to us getting stuck in an state where we think we should unprefix certain styles. [Describe test coverage new/current, TreeHerder]: Test included with patch. Existing test coverage is pretty good IMO (but didn't cover this use-case). [Risks and why]: Low-risk. This just tightens the restriction on when we activate a particular unprefixing feature, and this feature is already restricted to a small set of sites. [String/UUID change made/needed]: None

(In reply to Daniel Holbert [:dholbert] from comment #8) > [User impact if declined]: Potential for broken layout, *if* a user visits > any of the sites on our unprefixing whitelist, and that site happens to set > anyElement.style.display to "-webkit-box" using JavaScript. (The good news is that I haven't actually encountered any of the sites on our whitelist actually doing this & triggering this bug, FWIW, but it's conceivable that they might in some subsection of a site that I haven't tested; and it's also conceivable that they might change to do so during a website edit & start triggering this bug.)

Comment on attachment 8609488 [details] [diff] [review] fix v2 (now with test & commit message) Has tests, early in the cycle. Taking it for 40. Liz will make the call for 39.

Comment on attachment 8609488 [details] [diff] [review] fix v2 (now with test & commit message) Approved for uplift to aurora, sounds low-risk; we don't want broken layouts

Thanks!

(In reply to Liz Henry (:lizzard) from comment #13) > Approved for uplift to aurora, sounds low-risk; we don't want broken layouts (I'll assume you meant to say "beta"; this comment was granting approval-mozilla-beta+. :))