Last Comment Bug 692071 - The Context Menu and Awesomelist are dismissed in the same time, if a tap is performed outside the Context Menu UI
: The Context Menu and Awesomelist are dismissed in the same time, if a tap is ...
Status: VERIFIED FIXED
: regression
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: Firefox 10
Assigned To: Lucas Rocha (:lucasr)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-05 04:48 PDT by Cristian Nicolae (:xti)
Modified: 2011-10-10 07:31 PDT (History)
9 users (show)
mbrubeck: in‑testsuite+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Keep awesome panel open when context menu is dismissed (2.51 KB, patch)
2011-10-05 07:57 PDT, Lucas Rocha (:lucasr)
mark.finkle: review+
Details | Diff | Review
(1/3) Keep awesome panel open when context menu is dismissed (2.52 KB, patch)
2011-10-06 07:03 PDT, Lucas Rocha (:lucasr)
lucasr.at.mozilla: review+
Details | Diff | Review
(2/3) Ensure awesome panel is closed before each awesome screen test (2.83 KB, patch)
2011-10-06 07:05 PDT, Lucas Rocha (:lucasr)
mark.finkle: review+
Details | Diff | Review
(3/3) Add browser test for popup being dismissed on top of awesome panel (1.81 KB, patch)
2011-10-06 07:06 PDT, Lucas Rocha (:lucasr)
mark.finkle: review+
Details | Diff | Review

Description Cristian Nicolae (:xti) 2011-10-05 04:48:47 PDT
Build ID: Mozilla/5.0 (Android;Linux armv7l;rv:9.0a2)Gecko/20111004
Firefox/9.0a2 Fennec/9.0a2
Device: Samsung Galaxy S
OS: Android 2.2

Build ID: Mozilla/5.0 (Android;Linux armv7l;rv:10.0a1)Gecko/20111004
Firefox/10.0a1 Fennec/10.0a1
Device: Samsung Galaxy S
OS: Android 2.2

Steps to reproduce:
1. Open Fennec App
2. Tap on URL Bar for Awesomescreen
3. Long tap on any page for the Context Menu
4. Tap outside the Context Menu UI to dismiss it

Expected result:
After step 4, the Context Menu is dismissed and the Awesomescreen remains on the screen.

Actual result:
After step 4, the Context Menu and the Awesomescreen are dismissed in the same time.
Comment 1 Aaron Train [:aaronmt] 2011-10-05 06:28:33 PDT
Aurora is also affected
Mozilla/5.0 (Android; Linux armv7l; rv:9.0a2) Gecko/20111005 Firefox/9.0a2 Fennec/9.0a2
Comment 2 Lucas Rocha (:lucasr) 2011-10-05 07:57:45 PDT
Created attachment 564856 [details] [diff] [review]
Keep awesome panel open when context menu is dismissed
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-05 13:15:33 PDT
Comment on attachment 564856 [details] [diff] [review]
Keep awesome panel open when context menu is dismissed

Why isn't handleEscape handling this correctly?

http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/browser-ui.js#850
Comment 4 Matt Brubeck (:mbrubeck) 2011-10-05 13:18:55 PDT
(In reply to Mark Finkle (:mfinkle) from comment #3)
> Why isn't handleEscape handling this correctly?

This bug happens when the popup is dismissed by tapping, not when it's dismissed by the escape key.
Comment 5 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-05 13:26:26 PDT
Comment on attachment 564856 [details] [diff] [review]
Keep awesome panel open when context menu is dismissed

* _hasPopup -> _popupShowing
* Should we worry that _popupShowing is not ref counted and could be messed up by more than 1 popup open?

r+, but fix the nit and think about the question

do we have any tests for this situation?
Comment 6 Cristian Nicolae (:xti) 2011-10-06 01:53:58 PDT
This issue doesn't occur on:
Build ID : Mozilla /5.0 (Android;Linux armv7l;rv:9.0a1) Gecko/20110902 Firefox/9.0a1 Fennec/9.0a1 
http://hg.mozilla.org/mozilla-central/rev/0664108eb19d

but it occurs on:
Build ID : Mozilla /5.0 (Android;Linux armv7l;rv:9.0a1) Gecko/20110903 Firefox/9.0a1 Fennec/9.0a1
http://hg.mozilla.org/mozilla-central/rev/472716252ea3

Possible range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0664108eb19d&tochange=472716252ea3
Comment 7 Lucas Rocha (:lucasr) 2011-10-06 03:42:18 PDT
(In reply to Mark Finkle (:mfinkle) from comment #5)
> Comment on attachment 564856 [details] [diff] [review] [diff] [details] [review]
> Keep awesome panel open when context menu is dismissed
> 
> * _hasPopup -> _popupShowing
> * Should we worry that _popupShowing is not ref counted and could be messed
> up by more than 1 popup open?
> 
> r+, but fix the nit and think about the question

My understanding of popup handling in browser is that there's always only one popup open at a time. When a new popup is pushed, the previous one is dismissed. So, we only need to care if there's a popup on top of awesome panel or not.

> do we have any tests for this situation?

No. Writing one now.
Comment 8 Lucas Rocha (:lucasr) 2011-10-06 07:03:45 PDT
Created attachment 565207 [details] [diff] [review]
(1/3) Keep awesome panel open when context menu is dismissed

Same patch with nits fixed. Keeping the review+.
Comment 9 Lucas Rocha (:lucasr) 2011-10-06 07:05:02 PDT
Created attachment 565208 [details] [diff] [review]
(2/3) Ensure awesome panel is closed before each awesome screen test
Comment 10 Lucas Rocha (:lucasr) 2011-10-06 07:06:17 PDT
Created attachment 565209 [details] [diff] [review]
(3/3) Add browser test for popup being dismissed on top of awesome panel
Comment 11 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-06 07:22:12 PDT
Comment on attachment 565209 [details] [diff] [review]
(3/3) Add browser test for popup being dismissed on top of awesome panel

Just add a comment about using setTimeout to simulate a long tap. We try to avoid using setTimeout with non-zero values, but in this case it's kinda needed.
Comment 13 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2011-10-06 08:33:00 PDT
Oops, a little bit too early to set that flag yet.
Comment 14 Matt Brubeck (:mbrubeck) 2011-10-06 08:36:27 PDT
No, you can go ahead and set in-testsuite+ when pushing to inbound -
https://wiki.mozilla.org/Inbound_Sheriff_Duty
Comment 16 Catalin Suciu [:csuciu] 2011-10-10 01:53:41 PDT
This issue is verified fixed on Nightly build:
Mozilla/5.0 (Android;Linux armv7l;rv:10.0a1)Gecko/20111009 Firefox/10.0a1 Fennec/10.0a1
Device: HTC Desire
OS: Android 2.2

Note You need to log in before you can comment on or make changes to this bug.