Last Comment Bug 672813 - content isn't focused after clicking the Go button (focus remains in the location bar)
: content isn't focused after clicking the Go button (focus remains in the loca...
Status: VERIFIED FIXED
[testday-20110930]
: regression
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: 6 Branch
: x86 All
: -- normal (vote)
: Firefox 8
Assigned To: :Gavin Sharp [email: gavin@gavinsharp.com]
:
Mentors:
Depends on:
Blocks: 658383
  Show dependency treegraph
 
Reported: 2011-07-20 08:35 PDT by Alice0775 White
Modified: 2011-09-30 18:17 PDT (History)
4 users (show)
gavin.sharp: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
affected


Attachments
patch (2.51 KB, patch)
2011-07-20 12:47 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details | Diff | Splinter Review
patch with additional cleanup (3.58 KB, patch)
2011-07-20 13:04 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
dao+bmo: review+
Details | Diff | Splinter Review
comments addressed (3.60 KB, patch)
2011-07-20 13:23 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details | Diff | Splinter Review

Description Alice0775 White 2011-07-20 08:35:37 PDT
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/953f9620f395
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110720 Firefox/8.0a1 ID:20110720030844
http://hg.mozilla.org/releases/mozilla-aurora/rev/579cbf7a9add
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a2) Gecko/20110720 Firefox/7.0a2 ID:20110720042003

Content will not be focused after I type URL or TEXT or Keywod & TEXT in LocationBar and click GO button.

*This does not happen if I Press ENTER instead of clicking GO button.
*This does not happen if I middele-mouse-click GO button.
*This does not happen if I Press Alt-ENTER.
*This does not happen if I click a Autocomplete item in pull down popup.


*This does not happen Firefox6.0beta build2 and Firefox5.0.1.

Reproducible: Always

Steps to Reproduce:

1. Start Firefox with clean profile.
2. Open new blank tab.
3. Input http://mozilla.jp/firefox/ in Location Bar.
4. Click GO button.
5. Check focused element. (Push DOWN ARROW key))

Actual Results: 
  Location Bar is still focused.

Expected Results: 
  Content should be focused.

Regression window(cahed m-c hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/d04cae0f5d22
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110523 Firefox/6.0a1 ID:20110523061111
Fails:
http://hg.mozilla.org/mozilla-central/rev/0ad1aae1c559
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110523 Firefox/6.0a1 ID:20110523073652
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d04cae0f5d22&tochange=0ad1aae1c559

Suspected:
e5663e0e20b3	Gavin Sharp — Bug 658383: ensure that we avoid inheriting the owner principal when clicking the Go button, r=dao
Comment 1 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-20 11:54:43 PDT
I'm a little bit confused here - the bug doesn't occur in Firefox6.0beta build2, but it regressed between Firefox/6.0a1 ID:20110523061111 and Firefox/6.0a1 ID:20110523073652?

So it existed for 6.0 at some point, but then was fixed there, but wasn't fixed on 7/8? That would be pretty odd.
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-20 12:47:58 PDT
Created attachment 547208 [details] [diff] [review]
patch

This is the minimal change to restore the previous behavior, but I'd like to simplify this logic. I'll attach an alternate patch.
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-20 13:04:35 PDT
Created attachment 547211 [details] [diff] [review]
patch with additional cleanup

My understanding is this:
- in this method, content.focus() and selectedBrowser.focus() are effectively the same thing (this method isn't called on background windows)
- for loads in new tabs, we want to focus the content *before the new tab opens* so that focus is on content when returning to the tab that triggered the load
- for other loads in the same tab, we just need to focus the content (before or after doesn't matter)

I think just unconditionally focusing the content before doing anything achieves all of these goals.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-20 13:11:51 PDT
Oh, I guess we might also need to focus again afterwards so that the new tab's content is focused?
Comment 5 Dão Gottwald [:dao] 2011-07-20 13:14:22 PDT
(In reply to comment #4)
> Oh, I guess we might also need to focus again afterwards so that the new
> tab's content is focused?

If nothing else, updateCurrentBrowser should do that.
Comment 6 Dão Gottwald [:dao] 2011-07-20 13:18:53 PDT
Comment on attachment 547211 [details] [diff] [review]
patch with additional cleanup

With e10s, 'content' doesn't exist in the chrome process, and even if it was made to work, it would probably be less efficient, so I think we want gBrowser.selectedBrowser.focus().
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-20 13:23:34 PDT
Created attachment 547220 [details] [diff] [review]
comments addressed
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-20 13:33:17 PDT
https://hg.mozilla.org/integration/fx-team/rev/ef4ff782b849
Comment 9 Alice0775 White 2011-07-20 13:42:31 PDT
(In reply to comment #1)
> I'm a little bit confused here - the bug doesn't occur in Firefox6.0beta
> build2, but it regressed between Firefox/6.0a1 ID:20110523061111 and
> Firefox/6.0a1 ID:20110523073652?
> 
> So it existed for 6.0 at some point, but then was fixed there, but wasn't
> fixed on 7/8? That would be pretty odd.

Sorry , My bad.
This also happen on 6.0beta build2.
Comment 10 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-20 14:57:14 PDT
OK, thanks - that matches my understanding :)
Comment 11 Tim Taubert [:ttaubert] 2011-07-21 03:54:09 PDT
http://hg.mozilla.org/mozilla-central/rev/ef4ff782b849
Comment 12 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-07-21 18:17:30 PDT
This also affects Firefox 6 and Firefox 7. I'm not sure whether it's worth fixing there, though.
Comment 13 Gabriela [:gaby2300] 2011-09-30 18:16:08 PDT
Verified fixed using Firefox 8.0b1

Note You need to log in before you can comment on or make changes to this bug.