Closed Bug 1102374 Opened 5 years ago Closed 5 years ago

Enable display:contents by default in non-RELEASE builds

Categories

(Core :: Layout, defect, P5)

defect

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: mats, Assigned: mats)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

No description provided.
Priority: -- → P5
Assignee: nobody → mats
Status: NEW → ASSIGNED
Attachment #8527753 - Flags: review?(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.)
(Any particular reason for the RELEASE_BUILD ifdefs, though?  It's certainly not what I expected from the summary of the bug.)
(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.
Summary: Enable display:contents by default → Enable display:contents by default in non-RELEASE builds
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?)
> 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.
Blocks: 1105369
(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?
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.
(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.
Attachment #8527753 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/mozilla-central/rev/265e01c7ff55
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Flags: qe-verify-
Depends on: 1144434
You need to log in before you can comment on or make changes to this bug.