1135796 - ActivityUtils incorrectly divides API levels

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

We check (Versions.features11Plus), but then proceed to use flags introduced in 16 and 14.

https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/util/ActivityUtils.java

Comment 1

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

Attachment: MozReview Request: bz://1135796/mcomella

/r/4375 - Bug 1135796 - Update ActivityUtils to use proper API levels. r=rnewman

Pull down this commit:

hg pull review -r aafbbb38566d3b6fe6cccf877c5462aaa1781cb2

Comment 2

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

Haven't tested on ICS to see if this regressed, but I'm not sure we have an ICS device around.

Maybe we hadn't seen this sooner because the 3.0.x-4.0.x has so few users and none of them bothered to go into full screen mode.

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 8570019 [details]
MozReview Request: bz://1135796/mcomella

https://reviewboard.mozilla.org/r/4373/#review3575

Ship It!

::: mobile/android/base/util/ActivityUtils.java
(Diff revision 1)
>          } else {

No need for else after return.

Comment 4

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

Found a device. NI self to test on it.

Comment 5

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

Before this patch, the system status bar is not hidden but no crash; after this patch it is. Huzzah.

Doesn't seem like a super important uplift to beta, but who knows how it affects HC-ICS-.

Comment 6

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

remote:   https://hg.mozilla.org/integration/fx-team/rev/6ad61d16c331
remote:   https://hg.mozilla.org/integration/fx-team/rev/176f46639237

Comment 7

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

Comment on attachment 8570019 [details]
MozReview Request: bz://1135796/mcomella

Will likely need a branch patch since I added ActivityUtils.isFullScreen. If the uplifter wants to handle this branch patch, just don't include the isFullScreen changes.

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]:
  Users on HC - ICS devices will not have the system status bar hidden when they enter fullscreen mode. A polish issue.
  May also crash by accessing an invalid field, but I can't repro locally.

[Describe test coverage new/current, TreeHerder]:
  None

[Risks and why]:
  Low, we're conforming to API levels and falling back to old features. We don't have devices to test all the API levels in between, however, so maybe something terrible will happen.

[String/UUID change made/needed]: None

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6ad61d16c331
https://hg.mozilla.org/mozilla-central/rev/176f46639237

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

It's early in beta still so let's take this now and ensure it doesn't have a wider impact to user if we *didn't*.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/aabde40b9764

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7ede0abc5a1c

Comment 12

[Catalin Suciu [:csuciu]]

Michael, can you please provide some instructions on how to reproduce this issue, so we can test your fix.

Comment 13

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

On HC-ICS devices, when other devices entered full screen mode, these devices remained in the normal viewing mode. To test this,

1) Go to a fullscreen page (e.g. people.mozilla.org/~atrain/mobile/tests/test.mp4)
2) Long-press the video and select fullscreen

Expected: System status bar dims
Actual: Status bar does not dim

After this patch, the expected happens. There was no crash because the Android fullscreen APIs use binary flags.

Comment 15

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

Attachment: MozReview Request: Bug 1135796 - Update ActivityUtils to use proper API levels. r=rnewman