769905 - Doorhanger popup on Twitter Mobile closes too quickly

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Monreal [:micmon]]

On the Twitter mobile pages (for example: https://mobile.twitter.com/ChainsDD), a doorhanger (location information?) is shown for a fraction of a second but it auto-closes right away so it cannot be interacted with.

Comment 1

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

Margaret - Do we need to play with the timeout of the doorhanger for this?

Comment 2

[:Margaret Leibovic]

I added some logging to investigate, and the issue here is that we're getting two Content:LocationChange events for https://mobile.twitter.com/*. I'm not sure what Twitter is doing to cause this. As a workaround, we could make sure to only call updatePopups() if the URI hasn't changed, but that would be a change that would apply to all notifications. That would also mean that the notification would reappear if you reload a page if you just tapped outside to dismiss it, but most notifications would reappear in that case anyway.

I'm hesitant to add a timeout for the geolocation notification, since that can lead to bugs where the notification from a previous page appears on a new page.

Comment 3

[:Margaret Leibovic]

(In reply to Margaret Leibovic [:margaret] from comment #2)
> I added some logging to investigate, and the issue here is that we're
> getting two Content:LocationChange events for https://mobile.twitter.com/*.
> I'm not sure what Twitter is doing to cause this. As a workaround, we could
> make sure to only call updatePopups() if the URI hasn't changed, but that
> would be a change that would apply to all notifications. That would also
> mean that the notification would reappear if you reload a page if you just
> tapped outside to dismiss it, but most notifications would reappear in that
> case anyway.

Actually, after thinking about this more, if you've dismissed a popup, then reloaded the page, we wouldn't call updatePopups(), so you couldn't get the popup back if you wanted it, and that would be unexpected.

It seems like what we really want is to keep the popup showing if it's already showing *and* the URI hasn't changed.

Comment 4

[:Margaret Leibovic]

Attachment: patch

This patch makes it so that we'll only try to remove doorhangers on location change if the doorhanger popup is hidden or if the tab's URL is actually changing.

Comment 5

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

Comment on attachment 638819 [details] [diff] [review]
patch

Makes sense

Comment 6

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a48058e9b9b3

Comment 7

[:Margaret Leibovic]

Comment on attachment 638819 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: if a site does some sort of scripted reload, doorhanger notifications can disappear before the user can address them; this happens on Twitter
Testing completed (on m-c, etc.): just landed on m-c
Risk to taking this patch (and alternatives if risky): low-risk extra check before deciding to remove doorhangers
String or UUID changes made by this patch: n/a

Comment 8

[:Margaret Leibovic]

(In reply to Margaret Leibovic [:margaret] from comment #7)

> Testing completed (on m-c, etc.): just landed on m-c

Er, I mean inbound.

Comment 9

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/a48058e9b9b3

Comment 10

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1eb8ae1b3c1a