Tapping on URL bar does not dismiss the tabs tray

RESOLVED FIXED in Firefox 35

Status

()

Firefox for Android
General
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: CristinaM, Assigned: Jeff Lu)

Tracking

({regression, reproducible})

35 Branch
Firefox 40
ARM
Android
regression, reproducible
Points:
---

Firefox Tracking Flags

(firefox34 unaffected, firefox35 verified, firefox37 unaffected, firefox38 affected, firefox39 fixed, firefox40 verified, fennec39+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Environment:
Device: Nexus 4 (Android 4.4.4)
Build: Firefox for Android 35.0a1 (2014-10-01)

Steps to reproduce:
1. Launch Fennec and open the tabs tray;
2. Tap the URL bar.

Expected results:
Tabs tray is closed.

Actual results:
Tabs tray remains opened, URL bar is focused and the keyboard is displayed.

Regression window
Last good: 2014-09-26
First bad: 2014-09-27

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9e3d649b80a2&tochange=818f353b7aa6
Last good revision: 68f76e3c5787
First bad revision: c56275d516ec

Pushlog:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=68f76e3c5787&tochange=c56275d516ec

c56275d516ec	Lucas Rocha — Bug 1056002 - Tint the status bar on KitKat+ (r=mfinkle)
3fb6805983a0	Lucas Rocha — Bug 1056002 - Merge picasso and nineoldandroids jars into one (r=nalexander)
Blocks: 1056002
tracking-fennec: --- → ?
Keywords: regression, reproducible
Assignee: nobody → lucasr.at.mozilla
tracking-fennec: ? → 35+
Created attachment 8501852 [details] [diff] [review]
Account for statusbar height when tint is enabled (r=mfinkle)
Comment on attachment 8501852 [details] [diff] [review]
Account for statusbar height when tint is enabled (r=mfinkle)

When tint manager is enabled, the toplevel layout uses auto-padding to shift the UI within the content area (below the statusbar). We need account for that when tracking touch events in the browser chrome when the tabs tray is open.
Attachment #8501852 - Flags: review?(mark.finkle)

Updated

4 years ago
Duplicate of this bug: 1079978
Attachment #8501852 - Flags: review?(mark.finkle) → review+
Lucas, I know you duped bug 1079978 to this, will this also fix the dead-spot next to the dead-spot in the open tabs tray where it opens the menu?
(In reply to Aaron Train [:aaronmt] from comment #5)
> Lucas, I know you duped bug 1079978 to this, will this also fix the
> dead-spot next to the dead-spot in the open tabs tray where it opens the
> menu?

Yes, that should be fixed too.

Updated

4 years ago
Duplicate of this bug: 1080656
https://hg.mozilla.org/mozilla-central/rev/d0a4eacacc45
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
(Reporter)

Comment 10

4 years ago
Verified as fixed on
Build: Firefox for Android 35.0a1 (2014-10-10)
Device: Nexus 4 (Android 4.4.4)
Status: RESOLVED → VERIFIED
status-firefox35: affected → verified

Comment 11

4 years ago
This isn't fixed correctly. You can no longer enter the URL field from an open tab drawer. If the tab drawer is open and a user taps on the URL field, the drawer should close and the keyboard should open in a single action. Currently you're required to tap twice. First tap closes and another is required to enter the URL field.
(In reply to Paul [pwd] from comment #11)
> This isn't fixed correctly. You can no longer enter the URL field from an
> open tab drawer. If the tab drawer is open and a user taps on the URL field,
> the drawer should close and the keyboard should open in a single action.
> Currently you're required to tap twice. First tap closes and another is
> required to enter the URL field.

This is not what this bug is about. This bug is about regression that change UI behaviour unintentionally (side effect of the statusbar tinting). This patch reverts the behaviour to how it used to be.

Please file a bug with your suggested behaviour and we can discuss this with UX folks.

Updated

4 years ago
Duplicate of this bug: 1081155
This was backed out with bug 1056002 comment 26.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Renom: this is not fixed after the backout (and its uplift) of bug 1056002.
tracking-fennec: 35+ → ?
status-firefox39: --- → affected
Assignee: lucasr.at.mozilla → michael.l.comella
tracking-fennec: ? → 39+
Yeah- tapping the Toolbar UI when the tab tray is active, should bring the tab back up. Currently not getting that myself.
We dismiss the open TabsPanel from the touch event listener (HideOnTouchListener) attached to @id/main_layout (which contains the toolbar view). However, onInterceptTouchEvent is not called when the TabsPanel is open and the toolbar is pressed - I'm not yet sure why.
Curiously, GeckoApp/BrowserApp/TwoWayView.onInterceptTouchEvent gets called when the open TabsPanel is pressed, but not the toolbar when the TabsPanel is open.
(Assignee)

Comment 19

3 years ago
It appears that the touch is consumed by OuterLayout.onInterceptTouchEvent.[1]
> if (mIsOpen && mDragCallback.canInterceptEventWhileOpen(event)) { return true; }
But that should happen only if dragging (bug #909434) is enabled, so it seems we just need to move a bracket to expand the existing if(mDragCallback.canDrag()) block.

I can put it into a quick patch, if it helps.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/OuterLayout.java#159

Updated

3 years ago
status-firefox40: --- → affected
(Assignee)

Comment 20

3 years ago
Created attachment 8595684 [details] [diff] [review]
bug-1076692.patch

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8cd46cadd80e
Attachment #8595684 - Flags: review?(michael.l.comella)
Comment on attachment 8595684 [details] [diff] [review]
bug-1076692.patch

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

Makes sense to me! Thanks, Jeff!

Do you mind setting an "approval-mozilla-aurora" flag on this bug? It's tracking the version 39 release of Firefox, which is currently our aurora release, so we'll want to uplift it there.
Attachment #8595684 - Flags: review?(michael.l.comella) → review+
Assignee: michael.l.comella → jll544
For the record, from comment 15 on is actually a different issue with the same symptoms as the original purpose of this bug - it probably should have been filed into a separate bug so we could retain the original bug history.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed, leave-open
(Assignee)

Updated

3 years ago
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/202a071fa1a0
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
(Assignee)

Comment 25

3 years ago
Comment on attachment 8595684 [details] [diff] [review]
bug-1076692.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 909434
[User impact if declined]: Users are unable to tap the URL bar to close the tabs tray.
[Describe test coverage new/current, TreeHerder]: Tested locally.
[Risks and why]: Low. The patch further disables code for a disabled feature. See bug 909434, comment 70.
[String/UUID change made/needed]: None.
Attachment #8595684 - Flags: approval-mozilla-aurora?
Thanks for swooping in and taking this, Jeff!

If you're looking for a follow-up, bug 1025866 is a challenge and bug 1155911 if you want to do some digging (note that this last one is a bit time sensitive). Both are assigned, but just comment in the bug and we'll assign it to you.
(In reply to Michael Comella (:mcomella) from comment #26)
> but just comment in the bug and we'll
> assign it to you.

Actually, you should have the ability to assign yourself to bugs now - just assign the bug to yourself, and make a comment that I said it was okay. :)
If bug 909434 caused the regression than Firefox 38 is probably also affected.
status-firefox37: --- → unaffected
status-firefox38: --- → affected
Michael, or, is 38 not affected, since the regression may actually be bug 1056002?
status-firefox38: affected → ?

Comment 30

3 years ago
Verified as fixed in build 40.0a1 (2015-04-28);
Device: Nexus 4 (Android 4.4.4).
status-firefox40: fixed → verified
(In reply to Liz Henry (:lizzard) from comment #29)
> Michael, or, is 38 not affected, since the regression may actually be bug
> 1056002?

I can repro on 38. There are two separate problems here and they *should* be separate bugs (see comment 22) but they accidentally got merged here. The original issue was caused by bug 1056002, and was fixed here. I backed out both bug 1056002 and this bug. However, bug 909434 landed before the backout and caused a new issue with the same symptoms. The latest patch in this bug handles the issue caused by bug 909434.
status-firefox38: ? → affected
Comment on attachment 8595684 [details] [diff] [review]
bug-1076692.patch

Approved for uplift to aurora since this looks ok on mozilla-central and it should fix a UI regression.
Attachment #8595684 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Target Milestone: Firefox 35 → Firefox 40
You need to log in before you can comment on or make changes to this bug.