500490 - when viewing the larry ui, you can scroll the webpage and end up with the larry ui in the middle of the page

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

this is probably a simple fix, but on my maemo and winmo device I click the larry icon, then scroll the webpage down.  the larry information stays on the screen, but the title bar goes away...looks like something was missed.

Comment 1

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

The Larry panel should be listening for "blur" and will close on it's own. This seems to work if you just click somewhere in content. But if you pan, it does not work.

Comment 2

[Ben Combee (:bcombee)]

yeah, panning doesn't change the focus since the events are absorbed.  Perhaps we need to hook up "startPanning" to the dialog manager code.

Comment 3

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

(In reply to comment #2)
> yeah, panning doesn't change the focus since the events are absorbed.  Perhaps
> we need to hook up "startPanning" to the dialog manager code.

Or we maybe we could "focus()" the web document, even though we don't send the mousedown/click

Comment 4

[Ben Combee (:bcombee)]

Oooh, I like that idea, but would it possibly cause the softkeyboard to appear?  I guess if we give focus to the top-level document, it's OK.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

This is more global than the larry ui apparently, when we go to  http://www.mozilla.com/en-US/firefox/geolocation/ we can reproduce the problem with the geolocation prompt.

Oh, and when you pan, click on 'Give it a try', the prompt can be offscreen :s

Maybe we just want to freeze the differents panels at a fixed x/y position when we show them?

Comment 6

[Ben Combee (:bcombee)]

I tried putting focus on the web document, but it caused other issues, including some null references.  I feel Larry should go away when you interact with other things, but the geolocation prompt is more like the remember password prompt where it stays until you change page or dismiss it.

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #6)
> I tried putting focus on the web document, but it caused other issues,
> including some null references.  I feel Larry should go away when you interact
> with other things, but the geolocation prompt is more like the remember
> password prompt where it stays until you change page or dismiss it.

Yeah, you should be right, we need different behavior for different stuff, correct me if I'm wrong :
 * Larry ui should go away when we interact with something
 * Password prompt should stay until the user actively dismiss it
 * Geolocation prompt should go as soon you change page (what it don't do actually but  it's what it's done in Firefox)
 * Popup notification should go away when page change
 * Not sure of what we want to do with the helperApp ? (it can only be dismiss by the user now, it overlap awesome bar results, etc...)

Maybe I better open a new bug about the differents notification behavior?

Comment 8

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

Somewhat a dupe of bug 491811?

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

sounds like. I suppose I should wait Stuart/froygstig changes before diving into WidgetStack?

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.1

Comment 12

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

Comment on attachment 394269 [details] [diff] [review]
Patch v0.1

>diff -r 0b08c1c46854 chrome/content/browser-ui.js
>--- a/chrome/content/browser-ui.js	Wed Aug 12 22:10:58 2009 -0400
>+++ b/chrome/content/browser-ui.js	Thu Aug 13 16:47:37 2009 +0200
>@@ -240,16 +240,36 @@ var BrowserUI = {
>     if (this._dialogs.length)
>       this._dialogs.pop();
> 
>     // If no more dialogs are being displayed, remove the attr for CSS
>     if (!this._dialogs.length)
>       document.getElementById("toolbar-main").removeAttribute("dialog");
>   },
> 
>+  setContextualPanel: function(aPanel, aElements) {

* let's call it "pushPopup" so it kind of matches the "pushDialog"
* let's only pass in aElement (the root element of the panel), we don't need an array (more below)

>+    this._contextualPanel =  { "panel": aPanel, 

"_contextualPanel" -> "_popup"

>+  _updateContextualPanel: function _updateContextualPanel(aEvent) {

"_updatePopup"

>+    let element = this._contextualPanel.elements;
>+    let targetNode = aEvent.target;
>+    while (targetNode && element.indexOf(targetNode) == -1)
>+      targetNode = targetNode.parentNode;


Adjust this loop since this._popup.element is not an array

NOTE: I think you need a "popPopup" which just resets the this._popup

You need to call "popPopup" from the .hide() method of the popup panel _and_ from _updatePopup, if needed.

>+    window.addEventListener("mousedown", this, true);

Add a simple comment above this so we know it's for handling popups

>diff -r 0b08c1c46854 chrome/content/browser.js

>+    BrowserUI.setContextualPanel(this, [this._identityPopup, this._identityBox]);

We only need to track identityPopup, identityBox should close this already

NOTE: call BrowserUI.popPopup() from the .hide()

very close...

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

Address comments except the one concerning the array. 
In this case, for example, the _identityBox should normally close the _identityPopup, but, because the _identityPopup is dismissed before the click dispatch on _identityBox, instead of closing the _identityPopup, the 'click' open it again.

If I don't do that, I can try to do it like in Firefox with Panel and try to stop the event.

Comment 14

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

Comment on attachment 394395 [details] [diff] [review]
Patch v0.2

I don't love the need to supply an array, but we don't have a better solution for now.

Comment 15

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

pushed: https://hg.mozilla.org/mobile-browser/rev/bb3cd9687c13

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

verified with beta3

Comment 17

[Dão Gottwald [:dao]]

bugspam