Closed Bug 945777 Opened 8 years ago Closed 7 years ago

Enable position:sticky in gaia

Categories

(Firefox OS Graveyard :: Gaia, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed)

RESOLVED FIXED
1.4 S1 (14feb)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed

People

(Reporter: kgrandon, Assigned: kgrandon)

References

Details

(Keywords: perf, Whiteboard: [c=handeye p=1 s= u=1.3])

Attachments

(1 file)

As position:sticky is stabilizing we should consider enabling it for gaia. This will help with overall performance and code cleanliness.

This is also needed for the migration to APZ as scroll events will be fired less often.
We can review this one, but let's not land until we enable APZ.
Comment on attachment 8341786 [details] [review]
Pull request - enable position sticky pref

Don't you want to add it to preferences instead? In all cases it does not hurt since sticky is normally enabled by default on nightly build so do we really need it yet ?
Attachment #8341786 - Flags: review?(21)
This will enable position:sticky for all apps and web pages, correct?  Just curious if we want to open it up that wide given that there are still known crashers in position:sticky.  See the deps for bug 916315.

Given that there are stability concerns it would seem prudent to enable this at the beginning of a release cycle right after branching the previous release.  That would give us the maximum amount of burn-in time.
(In reply to Ben Kelly [:bkelly] from comment #4)
> This will enable position:sticky for all apps and web pages, correct?  Just
> curious if we want to open it up that wide given that there are still known
> crashers in position:sticky.  See the deps for bug 916315.
> 
> Given that there are stability concerns it would seem prudent to enable this
> at the beginning of a release cycle right after branching the previous
> release.  That would give us the maximum amount of burn-in time.

Yes, the current patch will enable it for all apps/web pages. There was some discussion of limiting it to certified apps only in bug 943849. I would recommend that we enable position sticky whenever we land APZ (bug 909877). 

I believe we've committed to APZ in 1.3 already, but I'm not sure how flexible this list is: https://wiki.mozilla.org/B2G/Roadmap#Committed_1.3_Features
(In reply to Kevin Grandon :kgrandon from comment #5)
> I believe we've committed to APZ in 1.3 already, but I'm not sure how
> flexible this list is:
> https://wiki.mozilla.org/B2G/Roadmap#Committed_1.3_Features

According to the history it was added to the 1.3 committed list the evening of Nov 20, 2013.  That was 12 business days (10 if you count the US Thanksgiving holiday) before 1.3 code freeze.

I don't know what the state of APZ was when that decision was made, but we're now 3 days from code freeze.  We have open crashers against position:sticky.  The APZ code is still being actively worked to remove bugs (for example bug 945277).  From talking with Vivien our current scroll handler strategies don't appear to work well with APZ (we don't receive onscroll during kinetic panning, etc).  I know position:sticky is meant to deal with that for headers, but we have a ton of code that modifies the DOM via onscroll events.

I know all of these problems can and will be resolved, but it seems like we would have a better shot at a quality release if we rode the trains in 1.4 instead of crash landing in 1.3.
(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #3)
> Comment on attachment 8341786 [details] [review]
> Pull request - enable position sticky pref
> 
> Don't you want to add it to preferences instead? In all cases it does not
> hurt since sticky is normally enabled by default on nightly build so do we
> really need it yet ?

I suppose, I'm not really sure what the benefit of either one is yet still. I assumed that we would need to turn this on before position:sticky is enabled for production. 

I've updated the PR, but will wait to see if we want to turn this on early or not depending on when APZ lands.
Whiteboard: [c= p=1 s= u=] → [c=handeye p=1 s= u=]
Comment on attachment 8341786 [details] [review]
Pull request - enable position sticky pref

APZC is going to be turned on for 1.3, and there is no chance that it miss 1.4. I'm going to push for sticky in 1.4, let's land that as it will let us is there is some bugs with sticky.
Attachment #8341786 - Flags: review+
Landed: https://github.com/mozilla-b2g/gaia/commit/4bfe2353566ed2164a97d900649e60f434b0854a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
1.3+, blocked existing 1.3 blocker Bug 943849 via Bug 942460.
blocking-b2g: --- → 1.3+
Priority: -- → P1
Whiteboard: [c=handeye p=1 s= u=] → [c=handeye p=1 s= u=1.3]
Target Milestone: --- → 1.4 S1 (14feb)
Uplifted 4bfe2353566ed2164a97d900649e60f434b0854a to:
v1.3: 4d1514ab33665d81112479c92626e4addd41a0ba
You need to log in before you can comment on or make changes to this bug.