Last Comment Bug 697086 - Content permission notifications not dismissed on page dismiss
: Content permission notifications not dismissed on page dismiss
Status: VERIFIED FIXED
: feature
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P2 normal (vote)
: ---
Assigned To: Gian-Carlo Pascutto [:gcp]
:
Mentors:
Depends on:
Blocks: 694672
  Show dependency treegraph
 
Reported: 2011-10-25 07:00 PDT by Aaron Train [:aaronmt]
Modified: 2012-01-10 11:46 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
Nightly (10/25) (49.65 KB, image/png)
2011-10-25 07:00 PDT, Aaron Train [:aaronmt]
no flags Details
Patch 1. Dismiss popups on page changes (2.76 KB, patch)
2011-11-01 08:27 PDT, Gian-Carlo Pascutto [:gcp]
mark.finkle: review+
Details | Diff | Review
Patch v2. Dismiss popups on page changes (2.88 KB, patch)
2011-11-01 09:14 PDT, Gian-Carlo Pascutto [:gcp]
gpascutto: review+
Details | Diff | Review

Description Aaron Train [:aaronmt] 2011-10-25 07:00:56 PDT
Created attachment 569352 [details]
Nightly (10/25)

See screenshot

STR:
1. http://maps.google.com
2. http://google.com

--
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111025 Firefox/10.0a1
Samsung Galaxy SII (Android 2.3.4)
Comment 1 Gian-Carlo Pascutto [:gcp] 2011-11-01 08:27:57 PDT
Created attachment 571012 [details] [diff] [review]
Patch 1. Dismiss popups on page changes
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-01 08:42:42 PDT
Comment on attachment 571012 [details] [diff] [review]
Patch 1. Dismiss popups on page changes


>     void handleLocationChange(final int tabId, final String uri) {

>         tab.updateURL(uri);
>+        mAppContext.mDoorHanger.removeForTab(tabId);

Do we need to worry about calling this from a background thread?

r+, but think about the background thread question
Comment 3 Gian-Carlo Pascutto [:gcp] 2011-11-01 08:49:07 PDT
>Do we need to worry about calling this from a background thread?

It's calling PopupWindow.dismiss(), so yeah, it should be in a Runnable on the main thread.
Comment 4 Gian-Carlo Pascutto [:gcp] 2011-11-01 09:14:19 PDT
Created attachment 571028 [details] [diff] [review]
Patch v2. Dismiss popups on page changes

Carrying forward review.
Comment 5 Gian-Carlo Pascutto [:gcp] 2011-11-01 13:36:50 PDT
hg.mozilla.org/projects/birch/rev/473727215fbe
Comment 6 Aaron Train [:aaronmt] 2011-11-02 06:45:28 PDT
20111102040257
http://hg.mozilla.org/projects/birch/rev/2b9de9749c1e
Samsung Nexus S (Android 2.3.6)

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