513290 - Page loads do not respect visibility of Awesome Bar Results

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aakash Desai [:aakashd]]

Build Ids:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.3a1pre) Gecko/20090826 Fennec/1.0a3pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20090827
Fennec/1.0b4pre


Steps to Reproduce:
1. Start a page load ( try pages with heavy content to see this more easily like www.espn.com )
2. Click on the URL bar during page load to open up the awesome bar results
3. Type a search entry into the field while the page is loading
4. Wait until the page has finished loading

Actual Results:
The awesome bar results will be gone, the loaded page will show and the url bar will be include the search entry

Expected Results:
The awesome bar results should still be shown on top of the loaded page

Comment 1

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

I think this is related to places performance.  Very noticeable on WinMo builds.

Comment 2

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

Attachment: Patch

It's more related to us :)

Comment 3

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

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

Comment 4

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2a2pre) Gecko/20090916 Fennec/1.0a3

and

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.3a1pre) Gecko/20090916
Fennec/1.0a3

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.2a2pre) Gecko/20090916
Fennec/1.0b4pre

Comment 5

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

This patch broke search buttons (bug 517030)

Comment 6

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

Attachment: Patch v0.2

A new package full of regressions! - feel free to r- :)

Comment 7

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

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


> 
>+  setToolbarValue: function(aValue) {
>+    if (this.isToolbarOpen())
>+      this._edit.defaultValue = aValue;
>+    else
>+      this._edit.value = aValue;
>+  },

Rename setToolbarValue to _setURI(aCaption)and move it to above _editToolbar.

Also, rename _editToolbar to __editURI and setURI to updateURI

>+  isToolbarOpen: function isToolbarOpen() {
>+    return this._edit.popup.popupOpen;
>+  },

Rename this to isAutoCompleteOpen() and move it above showAutoComplete()

>   doButtonSearch : function(button) {

>-    var urlbar = this._edit;
>-    urlbar.open = false;
>-    var value = urlbar.value;
>+    // We don't want the button to look pressed for now
>+    button.parentNode.selectedItem = null;

Actually, we _do_ want to see the button press, so remove those lines

>+    
>+    let urlbar = this._edit;
>+    urlbar.reallyClosePopup();

Drop the "urlbar" variable and just use "this._edit"


I want the renaming so we better understand what these methods _do_ from their names. The _editToolbar, for example has been around a long time and has changed it's implementation enough that the name no longer fits.

Comment 8

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

Attachment: Patch v0.3

So finally this patch add:
 * prevent the awesome bar to close unexpectedly
 * open the urlbar = focus / close the awesome bar = blur
 * keep the search string while the awesomebar is open
 * prevent the search buttons to look pressed

Comment 9

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

by the way this also resolved bug 515988.

The patch should be regression free...

Comment 10

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

I didn't find any regression

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

Comment 11

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

on the 20099021 1.9.2 nightly build, this retains the awesome bar state, but I cannot continue to type in the url bar, and selecting an awesomebar result seems to have a really long lag.

Comment 12

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

That's just a lag :)

Comment 13

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

yeah, Verified on n810 with 1.9.2 nightly build 20090921

Comment 14

[Al Billings [:abillings - ex-MoCo]]

in+litmus+

https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=9767