User-Agent: Mozilla/5.0 (X11; Linux i686; rv:6.0a1) Gecko/20110506 Firefox/6.0a1 Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:6.0a1) Gecko/20110506 Firefox/6.0a1 The list of suggestions presented by the awesomebar does not update after I close a tab with the Ctrl-W shortcut. (see STR for details) Reproducible: Always Steps to Reproduce: 1. Open a blank tab. 2. Type something generic in the URL bar (e.g. 'a'). The suggestions list for 'a' pops up. 3. Open another blank tab (Ctrl-T). 4. Type something else into the URL bar (e.g. 'b'). The suggestions list for 'b' pops up. 5. Close the tab WITH THE CTRL-W SHORTCUT. The 'a' tab is now active. 6. Although 'a' is now in the URL bar, the awesomebar still suggests sites for 'b'. Actual Results: Suggestions for previously closed tab remain displayed. Expected Results: Suggestions are updated for newly visible tab. Reproduceable on this machine on Firefox 4.0 and nightly (06-May-2010).
yeah, seeing it with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110507 Firefox/6.0a1
I think the Expected results should be: Suggestions for the closed tab should not be visible. This is because the old auto suggest was already closed, and we do not currently maintain a stack of such information on if the auto suggest was open or not at tab creation time. In Firefox 3.* by following the same steps the auto suggest would not show up again on tab 1 after the 2nd tab is closed. I think we should work in the same way as FF3.*. When a new tab is created we are actually currently closing the auto suggest. You can verify this by typing in some text so auto suggest comes up, then creating a tab (Ctrl+T), then going back to the previous tab (CTRL+TAB)
Closing the popup after the tab close makes sense to me.
Thanks, I'll create a patch that way for review.
Created attachment 532476 [details] [diff] [review] Close auto suggset popup on urlbar when tab is closing The newer tab's auto complete should not be present on the previous tab if the user pressed Ctrl+W to close the tab This matches the expected results as posted in Comment #2
(In reply to comment #2) > In Firefox 3.* by following the same > steps the auto suggest would not show up again on tab 1 after the 2nd tab is > closed. I think we should work in the same way as FF3.*. Did you investigate why our behavior changed?
I didn't see any related code changed directly that would cause the bug. I looked back in the history 12 months for tabbrowser.xml but didn't see a reason why. Perhaps it could be even older than that? It could be some kind of focus related fix somewhere else as the autocomplete closes when focus goes away, maybe related to all the tabcandy changes. For example if you click the tab x with your mouse that didn't reproduce the regression. It seems to work like the 3.x series now though with the attached patch.
It's probably more likely to be something in autocomplete.xml or the underlying panel code that caused the behavior change. I'd just find a regression range using nightlies and the pushlog.
Thanks for the info Gavin, I'll try your suggestion and figure out when the regression was introduced.
Thanks for the suggestions Gavin, I learned how to track down regressions better now :) I narrowed down via manual binary search on the nightly builds that: Works: 2010-11-08 http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2010/11/2010-11-08-04-mozilla-central/firefox-4.0b8pre.en-US.win32.installer.exe Broken: 2010-11-09 http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2010/11/2010-11-09-07-mozilla-central/firefox-4.0b8pre.en-US.win32.installer.exe I then went through the push log and reviewed the commits in that range. The regression seems to have been introduced on 2010-11-09: Bug 565575 - Retain location bar focus when switching tabs https://bugzilla.mozilla.org/show_bug.cgi?id=565575 http://hg.mozilla.org/mozilla-central/rev/dda010000be1 I think the attached patch still makes sense in order to not affect Bug 565575, but please let me know how to proceed.
Ah right, that makes sense - the focus switch is what would have caused the panel to be dismissed. I wonder whether it would make more sense to have the panel be dismissed any time the URL bar value changes programmatically (i.e. from the value setter) rather than specifically on tab switch.
Seems logical to me. I'll give that a try instead and post a patch when done.
I guess the other option is to actually update the results popup as though the text was entered by the user. That would probably have to be a lower-level adjustment to the autocomplete widget itself, though, and I'm not sure offhand whether that would break people.
Your suggestion in Comment 11 I think is best for this reason: 1) Open a tab and navigate to slashdot.org (auto complete is not open) 2) Open another tab and type in a letter or two so autocomplete comes up 3) Ctrl+Tab to go to the previous tab 4) autocomplete would be displayed still but with updated content, but shouldn't be shown. We would have to store per tab info of if autocomplete is open or not.
Created attachment 532853 [details] [diff] [review] Close popup if text is changed programmatically This fix works better than the last as it fixes both tab closes and tab switches, whereas the old fix only handled tab closing.
Comment on attachment 532853 [details] [diff] [review] Close popup if text is changed programmatically Sorry, I think I misled you - this breaks navigating through the popup using the arrow keys, since the value setter is called when you select an entry. I think we should revert to a more isolated approach of closing the popup on tab switch (e.g. from updateCurrentBrowser in tabbrowser.xml).
No problem, I'll submit a new patch with your suggestion. I was thinking about doing a fix in updateCurrentBrowser as well but wanted to submit one in the setter for review.
Created attachment 533042 [details] [diff] [review] Close popup on updateCurrentBrowser This patch works for tab switching, tab closing, and down button works. Tried full screen and not full screen as well.
Comment on attachment 533042 [details] [diff] [review] Close popup on updateCurrentBrowser Might be good to add a comment saying that we need to explicitly close the popup if the URL bar will potentially retain focus (in case we somehow add some other codepath where it could stay focused here in the future). We should also have a test for this. A browser-chrome test should be easy to write - you could probably even just add a few lines to browser_tabfocus.js. Feel free to ping me on IRC if you need any advice.
Will do about the comment. I'll let you know if I have any questions about the test case. Thanks for the guidance.
Created attachment 533163 [details] [diff] [review] Explicitly close the popup if the URL bar retains focus - Added a comment describing why I am closing the popup - Added a browser-chrome test into browser/base/content/test/browser_tabfocus.js to ensure across tab switches where urlbar is focused it will close the popup when open
Hmm, the test seems to be failing for me on Mac, with the patch applied, even though I confirmed that the bug as described in comment 0 is fixed.
test was working for me but I'll retry it and if I can't reproduce I'll try on mac.
Created attachment 533871 [details] [diff] [review] If urlbar was focused on tab switching to, close popup Found the reason why the test was failing. I think it worked for me because I was testing out of objdir with a diff file. I could reproduce the test fail consistently. The reason the test was failing is that each tab saves if the urlbar was focused, so I needed to set the focus on the urlbar for the tab that will be switched to, not the tab that I was switching from.
Comment on attachment 533871 [details] [diff] [review] If urlbar was focused on tab switching to, close popup a couple of things: - nit: s/we need to// in the comment, and add a space after "//" - I think I changed my mind about putting this in browser_tabfocus. putting it in its own test file is cleaner (just copy a simple test like browser_bug598923.js, and add it to the Makefile.in) otherwise this looks great. I'll wait for an updated patch to r+
Created attachment 534093 [details] [diff] [review] Close popup when switching to tab that previously had urlbar focused - Added a new test file and put it in Makefile.in - Fixed whitespace and line length problems
Comment on attachment 534093 [details] [diff] [review] Close popup when switching to tab that previously had urlbar focused Review of attachment 534093 [details] [diff] [review]: -----------------------------------------------------------------
Comment on attachment 534093 [details] [diff] [review] Close popup when switching to tab that previously had urlbar focused perfect! thank you.
The problem is still reproducible for me on Mozilla/5.0 (Windows NT 6.1; rv:6.0a2) Gecko/20110525 Firefox/6.0a2 If I close the second tab with with ctrl+w, the search for the letter "b" is present in the previous tab. (the letter "a" search tab) and the dropdown menu is present. If I press the "Esc" key, to get rid of the dropdown menu, I end up with "b" search instead of "a" search.
Hi Vlad, could you try a nightly from 2011-05-26?
Works for me in the Nightly build 2011-06-01.
Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:7.0a1) Gecko/20110527 Firefox/7.0a1