Enable position:sticky in gaia

RESOLVED FIXED in Firefox OS v1.3

Status

P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

({perf})

unspecified
1.4 S1 (14feb)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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

Details

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

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 8341786 [details] [review]
Pull request - enable position sticky pref
Attachment #8341786 - Flags: review?(21)
(Assignee)

Comment 2

5 years ago
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.
(Assignee)

Comment 5

5 years ago
(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.
(Assignee)

Comment 7

5 years ago
(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.

Updated

5 years ago
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+
(Assignee)

Comment 9

5 years ago
Landed: https://github.com/mozilla-b2g/gaia/commit/4bfe2353566ed2164a97d900649e60f434b0854a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 10

5 years ago
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
status-b2g-v1.3: --- → fixed
You need to log in before you can comment on or make changes to this bug.