Closed Bug 1257938 Opened 8 years ago Closed 8 years ago

Remove CSS pref "layout.css.sticky.enabled"

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

We've been shipping position:sticky support enabled by default since mid-2014 (Firefox 32), as of bug 916315.

Seems unlikely that we'd opt to roll back support at this point, so we should remove the pref and the code that reacts to it.
(I'll have patches ready for this shortly; taking.)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Corey, since you CC'd yourself, maybe you'd be up for reviewing? :) If not, I'm happy to tag heycam or someone else.

This first patch just makes our automated tests assume that position:sticky is unconditionally supported (dropping pref-enabling annotations).  I've also dropped the reftest that checks whether the pref works.

(Note: As hinted by an XXXdholbert comment in this patch, my next patch will merge test_position_sticky.html together with its iframe helper-file, since the separation there is no longer needed.)
Attachment #8732363 - Flags: review?(corey)
This is basically just cut-n-paste from file_position_sticky.html into test_position_sticky.html.

I've removed the "SimpleTest.waitForExplicitFinish()", too -- that was only there because the pref-setting & the iframe load were asynchronous. Now, everything happens synchronously, so we don't need waitForExplicitFinish() anymore.
Attachment #8732365 - Flags: review?(corey)
This last patch actually removes the pref from the code.  After this patch, grep tells me there are no remaining mentions of "layout.css.sticky.enabled" in the codebase.
Attachment #8732367 - Flags: review?(corey)
This all seems fine to me, and Try was okay, but I'll take a closer look after the weekend.
Sounds good, thanks!
Comment on attachment 8732363 [details] [diff] [review]
part 1: Adjust automated tests to assume position:sticky is unconditionally supported

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

r=corey
Attachment #8732363 - Flags: review?(corey) → review+
Comment on attachment 8732365 [details] [diff] [review]
part 2:  Remove separation between test_position_sticky.html & its helper-file, now that it doesn't need to tweak a pref.

It took me a bit to remember what this test is doing and why it's a mochitest. Maybe update the comment as long as we're here to clarify this, something roughly like

/** Test for Bug 886646 - Offsets for sticky positioning, when accessed through
    getComputedStyle(), should be accurately computed. In particular,
    percentage offsets should be computed in terms of the scroll container's
    content box. */

r=corey anyway.
Attachment #8732365 - Flags: review?(corey) → review+
Comment on attachment 8732367 [details] [diff] [review]
part 3: Remove support for the "layout.css.sticky.enabled" pref (so we'll unconditionally support "position: sticky")

r=corey
Attachment #8732367 - Flags: review?(corey) → review+
Flags: in-testsuite-
Oups wrong keyword, sorry for the spam. This consists mostly in checking we don't speak about this pref on MDN.
I don't think there's any documentation needed here; it'd just add noise to MDN.

Our behavior isn't changing; we're just removing the ability to disable this feature.

The page where this feature (sticky positioning) is documented...
  https://developer.mozilla.org/en-US/docs/Web/CSS/position
...*does* mention the pref ("layout.css.sticky.enabled is"), but only in a footnote, regarding the period of time where it was enabled in Nightly/Aurora but not in release.

--> Removing dev-doc-needed. (If you disagree, please elaborate a bit about what you think should be documented & where.)  Thanks!
Flags: needinfo?(jypenator)
Keywords: dev-doc-needed
Flags: needinfo?(jypenator)
Blocks: old-prefs
You need to log in before you can comment on or make changes to this bug.