499547 - opening bookmark item in library does not switch focus to main window

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[(mostly gone) XtC4UaLL [:xtc4uall]]

STR:
- open library
- load any bookmark item either with double-left-click or context menu "open"/"open in new tab"

expected:
- focus is set to/switches to main window loading the bookmark item

actual
- focus is kept on library window, bookmark item is loading while main window is in background

range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1057ca8f2a91&tochange=524477378eb1
=> regressed by checkin of Bug 178324

Comment 1

[Neil Deakin]

Attachment: this should work

This was an unintentional change caused due to the comment that was here being misleading.

Comment 2

[Neil Deakin]

Attachment: add a check to an existing test which opens bookmarks

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 384421 [details] [diff] [review]
add a check to an existing test which opens bookmarks

>   case "tab":
>     var browser = w.getBrowser();
>     browser.loadOneTab(url, referrerUrl, null, postData, loadInBackground,
>                        allowThirdPartyFixup || false);
>     break;
>   }
...
>+  else {
>+    var browser = w.getBrowserFromContentWindow(w.content);

This redeclares 'browser', should probably use 'let' in both cases.

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 384421 [details] [diff] [review]
add a check to an existing test which opens bookmarks

>+  if (window == fm.activeWindow) {
>+    w.focus();

Err, my testing didn't reveal this for some reason, but I'm pretty sure w.focus() is meant to be w.content.focus(), and 'window' should be 'w'.

Comment 5

[Neil Deakin]

(In reply to comment #4)
> (From update of attachment 384421 [details] [diff] [review])
> >+  if (window == fm.activeWindow) {
> >+    w.focus();
> 
> Err, my testing didn't reveal this for some reason, but I'm pretty sure
> w.focus() is meant to be w.content.focus(), and 'window' should be 'w'.

w.content.focus() makes sense but the old code used 'window' not 'w'.

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #4)
> and 'window' should be 'w'.

Scratch that part! I forgot what this bug is about for a moment...

Comment 7

[Dão Gottwald [:dao]]

(In reply to comment #5)
> the old code used 'window' not 'w'.

Yep, window is the library (which is the active window) in this case, w is the background browser window.

Comment 8

[Neil Deakin]

Attachment: change to w.content

Comment 9

[Neil Deakin]

http://hg.mozilla.org/mozilla-central/rev/7f9ef4bef487

Comment 10

[Tracy Walker [:tracy]]

verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b3pre) Gecko/20091110 Namoroka/3.6b3pre

Comment 11

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv