1102374 - Enable display:contents by default in non-RELEASE builds

Status: RESOLVED FIXED

(Core :: Layout, defect, P5)

Comment 1

[Mats Palmgren (inactive)]

Attachment: Enable display:contents by default in non-release builds.

Comment 2

[Mats Palmgren (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=e208c85abbe4

Comment 3

[David Baron :dbaron:]

I think this should probably wait until after merge day (this Friday, November 28), so that you have time to react to any fuzzing bugs that turn up.  (Also, once it lands, please point Jesse at it.)

Comment 4

[David Baron :dbaron:]

(Any particular reason for the RELEASE_BUILD ifdefs, though?  It's certainly not what I expected from the summary of the bug.)

Comment 5

[Mats Palmgren (inactive)]

(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #3)
> I think this should probably wait until after merge day

Sure.

(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #4)
> (Any particular reason for the RELEASE_BUILD ifdefs, though?  It's certainly
> not what I expected from the summary of the bug.)

Re-summarized.  I'll file a separate bug later to remove the #ifdef.

Comment 6

[David Baron :dbaron:]

Comment on attachment 8527753 [details] [diff] [review]
Enable display:contents by default in non-release builds.

So what's the plan for keeping the tests passing when aurora merges to beta?

(That said, why not just pref it on at the beginning of the cycle with the intent to actually ship it?)

Comment 7

[Mats Palmgren (inactive)]

> So what's the plan for keeping the tests passing when aurora merges to beta?

The updated tests pass also with the pref value being false, if that's what
you're asking.

> (That said, why not just pref it on at the beginning of the cycle with the
> intent to actually ship it?)

It seems safer to remove the #ifdef when we think it's ready to ship.
Like we have done for numerous other new features.
http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#152
http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#420
http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#572
etc.

Comment 8

[David Baron :dbaron:]

(In reply to Mats Palmgren (:mats) from comment #7)
> It seems safer to remove the #ifdef when we think it's ready to ship.
> Like we have done for numerous other new features.
> http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#152
> http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#420
> http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#572
> etc.

Why do you think it's not ready to ship?

Comment 9

[Mats Palmgren (inactive)]

I didn't imply it's not ready to ship.  I have no opinion on that
since there isn't any data to make such a decision.  Getting that
data is the purpose of this patch.

The intent is to make a decision, based on data, near the end of
the 37-cycle whether we want to enable it by default in v37.
(Jet filed bug 1105369 on that.)

"#ifdef RELEASE_BUILD" seems to be the established procedure to 
enable fuzz testing on new features while having a safety pin to
avoid shipping something unintentionally.  I don't see why we
should treat this feature differently.

Comment 10

[David Baron :dbaron:]

(In reply to Mats Palmgren (:mats) from comment #9)
> "#ifdef RELEASE_BUILD" seems to be the established procedure to 
> enable fuzz testing on new features while having a safety pin to
> avoid shipping something unintentionally.  I don't see why we
> should treat this feature differently.

I think it's normally for features that we know aren't ready to ship.

That said, given the desire to have this fuzzed, I'm ok with making an exception here, but we should actually make sure to come back and enable it.

Comment 11

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/265e01c7ff55

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/265e01c7ff55

Comment 13

[Jean-Yves Perrier [:teoli]]

Added a comment in the compat table of 
https://developer.mozilla.org/en-US/docs/Web/CSS/display#Browser_compatibility
and in
https://developer.mozilla.org/en-US/Firefox/Releases/37#CSS