1047264 - Regression: On screen options button appears after exiting edit mode on devices with hardware options button

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Flaviu Cos]

Environment: 
Device: Samsung Galaxy Tab 3 7.0 (Android 4.1.2);
Build: Nightly 34.0a1 (2014-07-31);

Steps to reproduce:
1. Launch Firefox;
2. Tap on the URL bar to enter edit mode;
3. Tap on 'x' button on top right corner to exit edit mode.

Expected result:
Only the refresh button is displayed next to the URL bar.

Actual result:
The on screen options button appears next to the refresh button.

Notes:
This bug only refers to the devices that have hardware options button.

Comment 1

[Flaviu Cos]

Regression range:
Last good build: 2014-06-02;
First bad build: 2014-06-03;

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6a984e21c2ca&tochange=f28005b84ed0

Possible regression of bug 997477?

Comment 2

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

Attachment: Dynamically retrieve Views for Display Mode on tablet

Comment 3

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

Comment on attachment 8467333 [details] [diff] [review]
Dynamically retrieve Views for Display Mode on tablet

Decided I'd rather use a getter.

Comment 4

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

Comment on attachment 8467333 [details] [diff] [review]
Dynamically retrieve Views for Display Mode on tablet

Actually, the getter is strange because we need to ensure it throws an IllegalStateException if called in stopEditing before showEditing, but there's no state to do that. This can be fixed by fixing the UIMode state (e.g. UIMode.EDIT, DISPLAY, ANIMATING).

Alternatively, perhaps we should have a better method of determining which views are currently in use by a device.

Comment 5

[Lucas Rocha (:lucasr)]

Comment on attachment 8467333 [details] [diff] [review]
Dynamically retrieve Views for Display Mode on tablet

Review of attachment 8467333 [details] [diff] [review]:
-----------------------------------------------------------------

Ok.

Comment 6

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

https://hg.mozilla.org/integration/fx-team/rev/69409e193649

Comment 7

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

Comment on attachment 8467333 [details] [diff] [review]
Dynamically retrieve Views for Display Mode on tablet

Approval Request Comment
[Feature/regressing bug #]: 
  Cancel button in tablet toolbar

[User impact if declined]:
  Users on devices with hardware menu buttons will have an inconsistent experience - i.e. the menu button will appear 

[Describe test coverage new/current, TBPL]:
  Tested locally

[Risks and why]:
  * We dynamically determine which views need to be hidden on the toolbar on tablet, which means if we incorrectly determine this information, we may permanently hide/display certain views, providing an inconsistent user experience.
  * We dynamically determine these views when the awesomescreen is entered. If the browser somehow starts in the awesomescreen (an unexpected behavior), we will throw an exception and crash.

[String/UUID change made/needed]: N/A

Comment 8

[Aaron Train [:aaronmt]]

(In reply to Michael Comella (:mcomella) from comment #7)
> [Describe test coverage new/current, TBPL]:
>   Tested locally

Let's get this verified on tomorrow's Nightly and be on the lookout for any related problems before any uplift.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/69409e193649

Comment 10

[Flaviu Cos]

(In reply to Aaron Train [:aaronmt] from comment #8)
> Let's get this verified on tomorrow's Nightly and be on the lookout for any
> related problems before any uplift.

The bug is still reproducible in today's Nightly build (2014-08-06) using Samsung Galaxy Tab 3 7.0 (Android 4.1.2)

Comment 11

[Aaron Train [:aaronmt]]

(In reply to Flaviu Cos, QA [:flaviu] from comment #10)
> (In reply to Aaron Train [:aaronmt] from comment #8)
> > Let's get this verified on tomorrow's Nightly and be on the lookout for any
> > related problems before any uplift.
> 
> The bug is still reproducible in today's Nightly build (2014-08-06) using
> Samsung Galaxy Tab 3 7.0 (Android 4.1.2)

We'll try tomorrow again.

Comment 12

[Flaviu Cos]

Verified as fixed in build 34.0a1 (2014-08-07);
Device: Samsung Galaxy Tab 3 7.0 (Android 4.1.2)

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/518ed9988257
https://hg.mozilla.org/releases/mozilla-beta/rev/e8fbf14de311

Comment 14

[Aaron Train [:aaronmt]]

[Tracking Requested - why for this release]: