Closed
Bug 1038178
Opened 10 years ago
Closed 10 years ago
[Vertical Home screen] Use scrollbar from root scrollframe
Categories
(Firefox OS Graveyard :: Gaia::Homescreen, defect)
Tracking
(b2g-v2.1 fixed, b2g-v2.2 fixed)
RESOLVED
FIXED
2.1 S4 (12sep)
People
(Reporter: kgrandon, Assigned: kgrandon)
References
Details
(Whiteboard: [systemsfe])
Attachments
(2 files)
(deleted),
text/x-github-pull-request
|
cwiiis
:
review+
fabrice
:
approval-gaia-v2.1+
|
Details |
(deleted),
text/x-github-pull-request
|
kgrandon
:
review+
fabrice
:
approval-gaia-v2.1+
|
Details |
We want to fire native mozbrowserasyncscroll events for the vertical homescreen, in order to do so we need to stop using an inner scrollframe, and use the root scrollframe. This bug tracks that work.
Assignee | ||
Comment 1•10 years ago
|
||
Comment 2•10 years ago
|
||
Can I ask why you want mozbrowserasyncscroll events? We're hoping to phase those out entirely (bug 898075). They don't really solve the problem of getting low-latency notifications of async scrolling, because it still requires dispatching messages to the main thread which might be busy.
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> Can I ask why you want mozbrowserasyncscroll events? We're hoping to phase
> those out entirely (bug 898075). They don't really solve the problem of
> getting low-latency notifications of async scrolling, because it still
> requires dispatching messages to the main thread which might be busy.
In 2.0 we want to animate the rocketbar layout (UI lives in the system app), depending on the scroll state of the page. This is similar to how the current FxOS browser works. I suppose we just need some way of driving an animation, cross-process, based on scroll position.
Looking over that bug, it appears that we thought we would no longer use this, but I think we had plans to be using it just as much (if not more).
Comment 4•10 years ago
|
||
Can you use the scroll event instead of the mozbrowserasyncscroll event? They should have approximately the same latency.
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> Can you use the scroll event instead of the mozbrowserasyncscroll event?
> They should have approximately the same latency.
I'm not sure of the history, but I'm not sure why we have two different events? It should be possible to use, but I haven't done any experimentation with it yet.
Comment 6•10 years ago
|
||
In the distant past we fired scroll much less frequently and mozbrowserasyncscroll events much more frequently. Now they fire at pretty much the same rate.
Assignee | ||
Updated•10 years ago
|
No longer blocks: vertical-home-next
Assignee | ||
Updated•10 years ago
|
Summary: [Vertical Homescreen] Use scrollbar from root scrollframe → [Vertical Home screen] Use scrollbar from root scrollframe
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8455345 [details]
Github pull request
Chris - would you mind giving this a quick review? Thanks!
Attachment #8455345 -
Flags: review?(chrislord.net)
Comment 8•10 years ago
|
||
Comment on attachment 8455345 [details]
Github pull request
Code inspection looks good (some comments on the pull request) - I've not tested that this works, I'm slightly surprised/pleased that there are no required changes to GaiaGrid.
I'm assuming that it does work fine, if stuff breaks we need more tests :)
Attachment #8455345 -
Flags: review?(chrislord.net) → review+
Comment 9•10 years ago
|
||
Out of interest, why do we want to use mozbrowserasyncscroll events here? I'm for this change anyway as it simplifies the DOM, I'm just curious.
Comment 10•10 years ago
|
||
the scrollable element was added to show the grey scrollbar while users are scrolling on the right. Is it working with the body right now? I hope so, it is clearer without scrollable element
Comment 11•10 years ago
|
||
(In reply to Cristian Rodriguez (:crdlc) from comment #10)
> the scrollable element was added to show the grey scrollbar while users are
> scrolling on the right. Is it working with the body right now? I hope so, it
> is clearer without scrollable element
yes, the scrollbar draws for the body now (this was fixed in bug 995519).
Comment 12•10 years ago
|
||
Kevin, be careful if the bug 1062820 is landed before this one because I added there a new integration test. Although I would prefer to wait here for bug 1062820 because it should be uplifted to 2.1 and the scrollable element lives in that branch and the cherry pick failed done directly because of this new integration test. If you cannot wait I will modify the integration test for 2.1 if this bug in landed before mine. Thanks a lot
Comment 13•10 years ago
|
||
Please keep in mind that if we aren't able to fix bug 1043859 then there is a possibility that we might disable the root frame scrollbars again. Unlikely but something you should keep in mind.
Updated•10 years ago
|
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Cristian Rodriguez (:crdlc) from comment #12)
> Kevin, be careful if the bug 1062820 is landed before this one because I
> added there a new integration test. Although I would prefer to wait here for
> bug 1062820 because it should be uplifted to 2.1 and the scrollable element
> lives in that branch and the cherry pick failed done directly because of
> this new integration test. If you cannot wait I will modify the integration
> test for 2.1 if this bug in landed before mine. Thanks a lot
Sure, I will give you right of way and will wait for your bug to land first, then update the test.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #13)
> Please keep in mind that if we aren't able to fix bug 1043859 then there is
> a possibility that we might disable the root frame scrollbars again.
> Unlikely but something you should keep in mind.
This is needed to get the right rocketbar animations on the home screen, but I think it's a little late for us to get that into 2.1. I think we should land this, and maybe not request uplift to 2.1 until we see what happens in bug 1043859. I hope we can keep root frame scrollbars in 2.2 if we lose them in 2.1.
Comment 15•10 years ago
|
||
Kevin, the bug with the new integration test has been landed so I guess that you have to touch this line theoretically
https://github.com/mozilla-b2g/gaia/commit/24fb519181335ff2fcf3a32cf1b775685f9089cf#diff-f0f7f9beab9ed2c6a86ea73d3c90779bR36
Assignee | ||
Comment 16•10 years ago
|
||
Thanks Cristian! The change seems simple, I've updated the pull request and will wait to see what travis says.
Assignee | ||
Comment 17•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 18•10 years ago
|
||
The patch made icons uncontrollable in edit mode. It's normal in the previous commit 8746bc86466677c3e1ed019c58c288577017362e .
Flags: needinfo?(kgrandon)
Assignee | ||
Comment 19•10 years ago
|
||
(In reply to Yi-Fan Liao [:yifan][:yliao] from comment #18)
> The patch made icons uncontrollable in edit mode. It's normal in the
> previous commit 8746bc86466677c3e1ed019c58c288577017362e .
Thanks, I'm looking into it now.
Assignee | ||
Comment 20•10 years ago
|
||
Here is a simple follow-up which uses the document scroll frame for calculations. r=me.
Attachment #8488423 -
Flags: review+
Flags: needinfo?(kgrandon)
Assignee | ||
Comment 21•10 years ago
|
||
Comment 22•10 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #20)
> Created attachment 8488423 [details]
> Follow-up - Use document scroll frame for drag drop
>
> Here is a simple follow-up which uses the document scroll frame for
> calculations. r=me.
Sorry I missed this, it was something I meant to check...
Updated•10 years ago
|
Target Milestone: --- → 2.1 S4 (12sep)
Comment 23•10 years ago
|
||
Comment on attachment 8455345 [details]
Github pull request
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 1064918
[User impact] if declined: Janky experience when rearranging icons on the homescreen
[Testing completed]: Been on master a long time
[Risk to taking this patch] (and alternatives if risky): If bug 995519 is disabled, scrollbars will not appear on the home screen
[String changes made]: None
Attachment #8455345 -
Flags: approval-gaia-v2.1?
Comment 24•10 years ago
|
||
Comment on attachment 8488423 [details]
Follow-up - Use document scroll frame for drag drop
[Approval Request Comment] See comment #23
Attachment #8488423 -
Flags: approval-gaia-v2.1?
Updated•10 years ago
|
Attachment #8455345 -
Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Updated•10 years ago
|
Attachment #8488423 -
Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Comment 25•10 years ago
|
||
v2.1: https://github.com/mozilla-b2g/gaia/commit/9f1b84c3655128aec5b50523878b7892f69430a0
v2.1: https://github.com/mozilla-b2g/gaia/commit/c91588fbddfe4a15418b2cf51e0792092eaed71c
status-b2g-v2.1:
--- → fixed
status-b2g-v2.2:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•