928800 - Doorhanger not anchored on awesomebar (Tablets)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

Attachment: Screenshot example

Nexus 7 (2013), portrait mode

Use a site that triggers a password storage Doorhanger. The Doorhanger sticks to the top of the screen instead of the Awesomebar.

Comment 1

[Aaron Train [:aaronmt]]

Rotation prior? Bug 739205.

Comment 2

[Gian-Carlo Pascutto [:gcp]]

The device was held in portrait mode throughout.

Comment 3

[:Margaret Leibovic]

This looks like what we do when the anchor view (the view that hold the favicon) is null.

Is this a recent regression, or does this happen on all release channels?

Comment 4

[Gian-Carlo Pascutto [:gcp]]

This doesn't reproduce on Nightly, but it does reproduce on Beta.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Hmm, it seems the STR aren't 100% reliable. I just managed to reproduce it on Nightly in Landscape mode, but I don't know what exact conditions trigger this.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

Margaret, is there anything I can log that would help nail this down? I can reproduce it say 50% of the time.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Just reproduced it on the gum test page on my Tab 10.1/3.2, with a gum permissions doorhanger.

Comment 8

[Sriram Ramasubramanian [:sriram]]

This usually happens when the anchor is not available/hidden. Is it possibility due to dynamic toolbar? I fixed that issue long back.

@Margaret: Aren't you taking care of doorhangers?

Comment 9

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

(In reply to Sriram Ramasubramanian [:sriram] from comment #8)

> @Margaret: Aren't you taking care of doorhangers?

This is on Fx26 and Margaret is booked with some other work at the moment.

Comment 10

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Whenever there is a change in the dynamic toolbar, if the doorhanger is shown, it's position is updated. Calling just update() does nothing useful for us. Instead, I'm calling show(), which will either update the location or show as drop-down depending on whether it is showing or not.

Comment 11

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

Comment on attachment 832615 [details] [diff] [review]
Patch

Let's make sure phones and tablets are both working fine with this patch.

Comment 12

[:Margaret Leibovic]

Looks like Sriram has this under control.

Comment 13

[Sriram Ramasubramanian [:sriram]]

And Sriram pushes the patch too!
https://hg.mozilla.org/integration/fx-team/rev/46c21551ce22

Comment 14

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 832615 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Dynamic toolbar.
User impact if declined: Doorhanger wont be anchored properly.
Testing completed (on m-c, etc.): Landed in fx-team today.
Risk to taking this patch (and alternatives if risky): This patch updates the location based on dynamic toolbar's visibility. If the dynamic toolbar shows up when doorhanger is showing, it realigns itself. If has a low risk.
String or IDL/UUID changes made by this patch: None.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/46c21551ce22

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0896f9e5c3f7
https://hg.mozilla.org/releases/mozilla-beta/rev/c9b696397007

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/c9b696397007

Comment 18

[Gian-Carlo Pascutto [:gcp]]

Unless I'm seriously misunderstanding something, this bug is purely Android/Java and never affected B2G.

Comment 19

[Ioana Chiorean]

Tested with Galaxy Tab  (4.0) using:
Firefox 26 Beta 8 
Firefox 27.0a2
Firefox 28.0a1

I can not reproduce it both at password nor at Gum Test (share mic/camera). Marking this Verifed fixed.