Scrolled off title-bar is shown when prior it is hidden after an action or dismissal of use of the text-selection action-bar

VERIFIED FIXED in Firefox 28

Status

()

Firefox for Android
Text Selection
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: aaronmt, Assigned: wesj)

Tracking

({reproducible, testcase})

29 Branch
Firefox 29
ARM
Android
reproducible, testcase
Points:
---

Firefox Tracking Flags

(firefox28 verified, firefox29 verified, fennec28+)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Not sure if this is intentional.

Currently when the text-selection action-bar is dismissed, the title bar is shown. If we show the text-action action-bar coming from a title-bar is hidden state, on dismissal I would have expected to see the title-bar remain hidden.

Steps to reproduce:

i) Visit http://www.lipsum.com/feed/html 
ii) Scroll down to hide the title-bar
iii) Do a text-selection, and then dismiss it or perform an action on the selected text

Expected: Title-bar remains hidden
Actual: Title-bar is revealed again afterwards

--
Nightly|Aurora (12/12), LG Nexus 4 (Android 4.4.2)
(Reporter)

Updated

5 years ago
tracking-fennec: --- → ?
Ian, what do you want the behavior to be here?
tracking-fennec: ? → 28+
Flags: needinfo?(ibarlow)
Aaron's right, the title bar should remember and return to its state before action mode was launched.
Flags: needinfo?(ibarlow)
(Reporter)

Updated

5 years ago
Keywords: reproducible, testcase
Assignee: nobody → wjohnston
(Assignee)

Comment 3

5 years ago
Created attachment 8358688 [details] [diff] [review]
Patch v1

This is the simplest way to do this.
Attachment #8358688 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8358688 [details] [diff] [review]
Patch v1

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

Looks ok. It would be nice if we could factor out the dynamic toolbar / action mode logic into a separate/reusable class.

::: mobile/android/base/BrowserApp.java
@@ +2589,1 @@
>          mViewFlipper.showPrevious();

nit: add empty line here.

@@ +2589,5 @@
>          mViewFlipper.showPrevious();
> +        // Only slide the urlbar out if it was hidden when the action mode started
> +        // Don't animate hiding it so that there's no flash as we switch back to url mode
> +        if (mShowActionModeEndAnimation) {
> +            margins.hideMargins(true);

Don't you ever need to reset mShowActionModeEndAnimation to false after using it?
Attachment #8358688 - Flags: review?(lucasr.at.mozilla) → review+
(Assignee)

Comment 6

5 years ago
Comment on attachment 8358688 [details] [diff] [review]
Patch v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 768667
User impact if declined: Titlebar scrolls on automatically but has to be manually scrolled off
Testing completed (on m-c, etc.): landing soon
Risk to taking this patch (and alternatives if risky): Pretty low risk, but we could leave things as is with minimal effect on users.
String or IDL/UUID changes made by this patch: None.
Attachment #8358688 - Flags: approval-mozilla-aurora?
Triage drive-by: waiting for this to get to Nightly from project branch.
I'm not sure how this was missed, but it landed on m-c a few days ago:
https://hg.mozilla.org/mozilla-central/rev/94dcc39391bc
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
status-firefox29: affected → fixed
Target Milestone: --- → Firefox 29

Updated

5 years ago
Attachment #8358688 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on:
Build: Firefox for Android 28 Beta 8 
Device: Samsung Galaxy Tab
OS: Android 4.0.4
status-firefox28: fixed → verified
Verified fixed on:
Build: Firefox for Android 29.0a2 (2014-03-05) 
Device: Alcatel One Touch 
OS: Android 4.1.2
Status: RESOLVED → VERIFIED
status-firefox29: fixed → verified
You need to log in before you can comment on or make changes to this bug.