Closed Bug 876741 Opened 12 years ago Closed 11 years ago

Scrollbars are not drawn on b2g

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:koi+, firefox26 wontfix, firefox27 wontfix, firefox28 fixed, b2g-v1.2 fixed)

Status:
RESOLVED FIXED
Milestone:
1.3 Sprint 5 - 11/22
Project Flags:
blocking-b2g koi+
Tracking Flags:
Tracking Status
firefox26 --- wontfix
firefox27 --- wontfix
firefox28 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: vingtetun, Assigned: vingtetun)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

bug876741.patch
5.37 KB, patch
roc
: review+
Details | Diff | Splinter Review
Attached patch Patch (obsolete) — — Splinter Review
It seems like the prefs for it has changed.
Attachment #754868 - Flags: review?(poirot.alex)
Comment on attachment 754868 [details] [diff] [review] Patch Review of attachment 754868 [details] [diff] [review]: ----------------------------------------------------------------- I think we should still keep `ui.showHideScrollbars` as there is still one usage of it: http://mxr.mozilla.org/mozilla-central/source/layout/style/AnimationCommon.cpp#344
Attachment #754868 - Flags: review?(poirot.alex) → review+
This stuff is sitting in my patch queue for a long time. It feels like turning it on again slow down panning because the scrollbar is repainted way too often. Milan do you know who can have a look at it? Steps to reproduce: - add pref("ui.showOverlayScrollbars", 1); in b2g/app/b2g.js - On the device go to Settings -> Informations -> More Informations -> Developer -> Show Repaints - Pan a little bit and observe that the scrollbar is repainted on every moves.
Flags: needinfo?(milan)
Seems like the perf issues are resolved with bug 813041
Depends on: 813041
Vivien, is the question still the same, after the comment 3?
(In reply to Milan Sreckovic [:milan] from comment #4) > Vivien, is the question still the same, after the comment 3? Nope. Sorry I forgot to remove the needinfo.
Talked with rel man about this - given that bug 813041 has existed on 1.01 already, we can ship another release with it present again. However, given that scrollbars have always been present across apps in 1.01 & 1.1, we need to turn them back on again, as it's a regression otherwise to have them off. On that regard, I'm pulling the dependency on bug 813041, as we do not need to wait on bug 813041 landing to turn the prefs back on. I'm also noming this since we need this back on in the 1.2 timeframe.
blocking-b2g: --- → koi?
No longer depends on: 813041
Keywords: regression
We need bug 813041 because the perfs sucks without the patch there. Thanks for keeping the dependency which is one of the reason why I have not landed this patch.
Depends on: 813041
Vivien based on conversation with :jason a) We need to enable scrollbars in 1.2, since we have had them since 1.0.1 and we would definitely need it for various use-cases (I think we all agree here ?) b) Your concern is that perf is impacted and is further worse in 1.2 if we enabled them(Correct ?). that's being tacked in 813041 c) Looks like https://bugzilla.mozilla.org/show_bug.cgi?id=813041#c13 the rebased patch works well. Is it just review away ? What's the risk there ? We would need to block on both if you confirm that the perf is worse and the upcoming patch helping the perf is low risk.Can you confirm ?
(In reply to bhavana bajaj [:bajaj] from comment #9) > Vivien based on conversation with :jason > > a) We need to enable scrollbars in 1.2, since we have had them since 1.0.1 > and we would definitely need it for various use-cases (I think we all agree > here ?) Yep. > b) Your concern is that perf is impacted and is further worse in 1.2 if we > enabled them(Correct ?). that's being tacked in 813041 Yes. > c) Looks like https://bugzilla.mozilla.org/show_bug.cgi?id=813041#c13 the > rebased patch works well. Is it just review away ? What's the risk there ? > I can not really tell for the risk for bug 813041. You may want to ask directly there, sorry :/
koi+ for regression.
blocking-b2g: koi? → koi+
(In reply to Preeti Raghunath(:Preeti) from comment #11) > koi+ for regression. This is tagged as depending on bug 813041 - if that is the case, that bug should be koi+ as well.
Sounds like this is in 1.2, so removing the "mozilla-central" from the summary to avoid confusion.
Summary: Scrollbars are not draw onb2g with mozilla-central → Scrollbars are not drawn on b2g
Attached patch bug876741.patch (obsolete) — — Splinter Review
Forwarding the r+ from ochameau after his comment.
Assignee: nobody → 21
Attachment #754868 - Attachment is obsolete: true
Attachment #830875 - Flags: review+
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #16) > Backed out for B2G reftest failures. > https://hg.mozilla.org/integration/b2g-inbound/rev/0db9bc1e8e39 > > https://tbpl.mozilla.org/php/getParsedLog.php?id=30482967&tree=B2g-Inbound > https://tbpl.mozilla.org/php/getParsedLog.php?id=30483514&tree=B2g-Inbound Sigh. Those are purely random since scrollbars appears automatically during reflows and dissapears after a while. So the image comparison depends on when the screenshot is done :(
I've added some random-if(B2G) and push to try to see if some other tests fails because of this randomess thing. https://tbpl.mozilla.org/?tree=Try&rev=27b8e930b0da
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #18) > I've added some random-if(B2G) and push to try to see if some other tests > fails because of this randomess thing. > https://tbpl.mozilla.org/?tree=Try&rev=27b8e930b0da humm. The more try tests I'm running the more random I see. I feel like this stuff may introduce a lot of random oranges just because the scrollbars are shown when there is a reflow and fades out with a timeout, that obviously is hard to get identical between the page and the reference page. :/ Not sure what's the best solution here. I more or less considering to disable this prefs during reftests (if doable).
Maybe my best chance is to fix those randoms by running tons of try builds and once there are green multiple times then this is good. :(
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #20) > Maybe my best chance is to fix those randoms by running tons of try builds > and once there are green multiple times then this is good. :( This run is Green https://tbpl.mozilla.org/?tree=Try&rev=ba7b77619dd9 I will re-run just to make sure to catch more random oranges.
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #17) > Sigh. Those are purely random since scrollbars appears automatically during > reflows and dissapears after a while. Really, scrollbars appear just because we did a reflow? I thought user interaction was required to trigger scrollbars on B2G. Is scrollbars appearing because we did a reflow desired behavior for some reason?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #24) > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #17) > > Sigh. Those are purely random since scrollbars appears automatically during > > reflows and dissapears after a while. > > Really, scrollbars appear just because we did a reflow? I thought user > interaction was required to trigger scrollbars on B2G. Is scrollbars > appearing because we did a reflow desired behavior for some reason? I think this is desireable from a UX point of view. It allows scrollbars to be displayed when a scrollable area is just created on the screen, letting the user knows that he/she can pan. At least this has been the rationale until today. I will land this patch and check with UX if this is something they want to change or not.
https://hg.mozilla.org/integration/mozilla-inbound/rev/20d8b156a140
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → 1.3 Sprint 5 - 11/22
https://hg.mozilla.org/mozilla-central/rev/20d8b156a140
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: