Closed
Bug 1132763
Opened 10 years ago
Closed 10 years ago
Hiding system UI in reader mode is broken
Categories
(Firefox for Android Graveyard :: Reader View, defect)
Tracking
(firefox35 affected, firefox36 affected, firefox37 fixed, firefox38 fixed, fennec37+)
People
(Reporter: Margaret, Assigned: mcomella, Mentored)
References
Details
(Keywords: regression, Whiteboard: [lang=java][good next bug])
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-review-board-request
|
Details |
Looks like a regression from bug 1111142.
Reporter | ||
Comment 1•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #0)
> Looks like a regression from bug 1111142.
Actually, this is broken for me on beta as well, so it seems like something regressed this earlier.
Reporter | ||
Updated•10 years ago
|
No longer blocks: 1111142
Keywords: regressionwindow-wanted
Reporter | ||
Comment 2•10 years ago
|
||
Looks like this is a regression in 35. AaronMT suspects bug 1081153.
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox37:
--- → affected
status-firefox38:
--- → affected
Updated•10 years ago
|
tracking-fennec: ? → +
Reporter | ||
Comment 3•10 years ago
|
||
I haven't looked into the code, but I would start by looking at the patch in bug 1081153 and seeing if backing that out fixes things.
Mentor: margaret.leibovic
Whiteboard: [lang=java][good next bug]
Comment 4•10 years ago
|
||
Let's try to address this sooner than later
Assignee: nobody → michael.l.comella
tracking-fennec: + → 37+
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #3)
> I haven't looked into the code, but I would start by looking at the patch in
> bug 1081153 and seeing if backing that out fixes things.
Didn't work for me - guess I'll have to do it the old fashioned way.
Assignee | ||
Comment 6•10 years ago
|
||
A few minutes of `hg grep --all setSystemUiVisibility` also seems to indicate that bug 1081153 was the only time this method has been modified recently.
I wonder where setFullscreen is called from for reader mode, if at all - it seems the only accesses are currently from JNI.
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #6)
> A few minutes of `hg grep --all setSystemUiVisibility` also seems to
> indicate that bug 1081153 was the only time this method has been modified
> recently.
Actually, I realized it just output my backout - go figure, `hg grep` takes too long.
Assignee | ||
Comment 8•10 years ago
|
||
The original implementation is in bug 960159.
`hg grep --all setSystemUiVisibility mobile/android/base` FTW!
Assignee | ||
Comment 9•10 years ago
|
||
The Java plumbing all seems to be in place but we only ever get visible == true to AboutReader._setSystemUIVisibility [1].
[1]: https://mxr.mozilla.org/mozilla-central/source/toolkit/components/reader/AboutReader.jsm?rev=1146999121f4#439
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #9)
> The Java plumbing all seems to be in place but we only ever get visible ==
> true to AboutReader._setSystemUIVisibility [1].
I lied - plumbing in place and we get false when scrolling downward (as opposed to entered reading mode, which is when I checked).
Assignee | ||
Comment 11•10 years ago
|
||
The navigation bar (i.e. back, home, recent apps) dims, but the status bar does not - apparently I've been looking in the wrong places.
I wonder if this has something to do with the custom system status bar color.
Assignee | ||
Comment 12•10 years ago
|
||
I backed out [1] from bug 1056002 and that fixed the issue.
[1]: https://hg.mozilla.org/integration/fx-team/rev/c56275d516ec
Blocks: 1056002
Keywords: regressionwindow-wanted
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #12)
> I backed out [1] from bug 1056002 and that fixed the issue.
Locally, that is.
Assignee | ||
Comment 14•10 years ago
|
||
/r/4125 - Bug 1132763 - Hide the system status bar when scrolling down in reader mode. r=margaret
Pull down this commit:
hg pull review -r 08f5b866ee9507822d22fbc560ec94eed2299802
Attachment #8567401 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8567401 [details]
MozReview Request: bz://1132763/mcomella
Nevermind, this seems to negatively affect the fullscreen APIs.
Attachment #8567401 -
Flags: review?(margaret.leibovic)
Assignee | ||
Updated•10 years ago
|
Blocks: fennec-fullscreen
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8567401 [details]
MozReview Request: bz://1132763/mcomella
/r/4125 - Bug 1132763 - Hide the system status bar when scrolling down in reader mode. r=margaret
Pull down this commit:
hg pull review -r e7e30e2e38132a5930743e45252328003255d58f
Attachment #8567401 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 17•10 years ago
|
||
This should be fixed by the backout of bug 1056002.
Assignee | ||
Updated•10 years ago
|
Attachment #8567401 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 18•10 years ago
|
||
Fixed by the uplift of bug 1056002 to 37.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 19•10 years ago
|
||
Attachment #8567401 -
Attachment is obsolete: true
Assignee | ||
Comment 20•10 years ago
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•