836689 - The security notification pop-up is still displayed on screen after going to Menu -> More -> Add-ons

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement)

Description

[Teodora Vermesan]

Firefox 19.0b4 (2013-01-31)
Device: HTC Desire Z
OS: Android 2.3.3

Steps to reproduce:
1. Open Firefox for Android
2. Go to gmail.com
3. Tap on the grey lock
4. Go to Menu -> More -> Add-ons

Expected result:
- The security notification triggered after step 3 is dismissed when you go to Menu->More->Add-ons

Actual result:
- The security notification is still displayed on screen after step 4

Comment 1

[Teodora Vermesan]

Attachment: screenshot

Comment 2

[Aaron Train [:aaronmt]]

The same happens for any web-site as the manager is web page.

Comment 3

[Mark Capella [:capella]]

Attachment: Patch (v1)

I didn't think this was the same thing as bug 759085 which is doorhanger related. This is SiteIdentity related ("Larry")...

This is dismissed during screen orientation change, touch outside the notification, back key, and etc. I've thought to add a dismiss when the menu is selected.

I also removed what appears to be an unrequired DoorHangerPopup #include I noticed while researching.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 708654 [details] [diff] [review]
Patch (v1)

I assume we want to dismiss the SiteIdentity popup because the site loaded in the browser is no longer the site that Larry is showing us information about?

If so, I think we should dismiss the site identity popup on page load, not just when opening the menu. We could just do Menu > Bookmark, which keeps the same site loaded.

Comment 5

[Mark Capella [:capella]]

I can follow clearing larry @ page open, allowing him to linger on-screen as long as logically consistent.

Doing |Menu > Bookmark| has me confused ... I don't understand the bookmark tie-in ...

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mark Capella [:capella] from comment #5)
> I can follow clearing larry @ page open, allowing him to linger on-screen as
> long as logically consistent.

That's the fix :)

> Doing |Menu > Bookmark| has me confused ... I don't understand the bookmark
> tie-in ...

This was my example of why we should _not_ only dismiss when showing the menu.

Comment 7

[Mark Capella [:capella]]

Ah :D as-in "We should allow for doing ..."

Comment 8

[Mark Capella [:capella]]

Attachment: Patch (v2)

This is a more tightly coupled / just-in-time approach for the specific situations where the issue can occur ...

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 708772 [details] [diff] [review]
Patch (v2)

Too tightly coupled.

I think you should be able to put the dismiss code right here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#104

That line should get hit if:
1. A tab is navigated to a new page.
2. The selected tab is changed.
3. A new tab is created.
4. A tab is removed.

Both of those triggers should probably mean we hide the SiteIdentity popup.

Comment 10

[Mark Capella [:capella]]

Attachment: Patch (v3)

Yep, this works for me ... it accomplishes the same result through a single code-path. Note that larry is cleared on a page refresh also, in the previous one I let him hang around for that :)

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 709019 [details] [diff] [review]
Patch (v3)

\o/

Comment 12

[Mark Capella [:capella]]

Try: https://tbpl.mozilla.org/?tree=Try&rev=5eb345af267f

Comment 13

[Mark Capella [:capella]]

On to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/42979cfc8b33

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/42979cfc8b33

Comment 15

[Andreea Pod]

Verified fixed on:
-build: Firefox for Android 21.0a1 (2013-02-10)
-device: HTC Desire Z
-OS: Android 2.3.3