1132763 - Hiding system UI in reader mode is broken

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Reader View, defect)

Description

[:Margaret Leibovic]

Looks like a regression from bug 1111142.

Comment 1

[:Margaret Leibovic]

(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.

Comment 2

[:Margaret Leibovic]

Looks like this is a regression in 35. AaronMT suspects bug 1081153.

Comment 3

[:Margaret Leibovic]

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.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's try to address this sooner than later

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(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.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(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.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

The original implementation is in bug 960159.

`hg grep --all setSystemUiVisibility mobile/android/base` FTW!

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(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).

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I backed out [1] from bug 1056002 and that fixed the issue.

[1]: https://hg.mozilla.org/integration/fx-team/rev/c56275d516ec

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #12)
> I backed out [1] from bug 1056002 and that fixed the issue.

Locally, that is.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: 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 08f5b866ee9507822d22fbc560ec94eed2299802

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8567401 [details]
MozReview Request: bz://1132763/mcomella

Nevermind, this seems to negatively affect the fullscreen APIs.

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

This should be fixed by the backout of bug 1056002.

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Fixed by the uplift of bug 1056002 to 37.

Comment 20

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1132763 - Hide the system status bar when scrolling down in reader mode. r=margaret