'Current Navigator Window' option in Open Web Location dialog is grayed out

VERIFIED FIXED in mozilla0.9.4

Status

()

P3
minor
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: shrir, Assigned: cmanske)

Tracking

Trunk
mozilla0.9.4
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
today's branch 0723

steps:
launch composer
go to 'File|Open Web Location'
In the dialog that opens up, pull down the combobox labled "Open in"
select 'New Navigator window' and now retry selecting 'Current Navigator window'

Observe that the option is grayed out and you cannot select it anymore.

Comment 1

17 years ago
handing over to cmanske, charley -- is this us or someone else?
Assignee: beppe → cmanske
Severity: normal → minor
Priority: -- → P3
Target Milestone: --- → mozilla1.0

Comment 2

17 years ago
Is there a current navigator window?  You're in a Composer window...
I would think that, at least sometimes, this menu item *should* be disabled.
Hardware: PC → All
(Assignee)

Comment 3

17 years ago
Kathy is right, but current code in openLocation.js disables the item all the
time. Technically not my bug, but it's clear how to fix it, so I'll take it.
Status: NEW → ASSIGNED
(Assignee)

Comment 4

17 years ago
Created attachment 43383 [details] [diff] [review]
Get current browser window when using dialog from Composer
(Assignee)

Comment 5

17 years ago
To verify:
1. Open Composer, close all browser windows.
2. Use Open Location dialog to open a URL into a "New Navigator window"
   ("Current Navigator window" menuitem should be disabled.)
3. Go back to composer window and use Open Location again, but "Current 
Navigator window" should be enabled - open another URL with that option.

I think Blake is the owner of this code now, so review is requested.
The only remaining problem I see is that focus is not shifted to browser 
window after the URL is loaded the second time (wheb "Current Navigator
window" was used.)
I tried this:
  setTimeout("browser.focus();", 100);
after the line:
  browser.loadURI(url);
but it didn't work
Keywords: patch, review
Whiteboard: FIXINHAND need r=, sr=

Comment 6

17 years ago
I think we need to change the string for that menu item/
To me, the "current window" is a Composer window, not a navigator window. 
 How about "Existing Navigator window"?  Other suggestions?
(Assignee)

Comment 7

17 years ago
It already is "Current Navigator window". I think that's fine.
Noting that, one could argue that current behavior is correct -- when you are in
Composer, there is no "current navigator".
(Assignee)

Comment 8

17 years ago
Robinf agrees with brade's suggested "Existing Navigator Window". 
That sounds good to me to avoid having to change the string only when 
launching the dialog from Composer.
Blake: This would change string for Navigator usage as well. Does this sound ok
to you?
(Assignee)

Updated

17 years ago
Target Milestone: mozilla1.0 → mozilla0.9.4
(Assignee)

Comment 9

17 years ago
Created attachment 44420 [details] [diff] [review]
Updated patch to include string change to "Existing Navigator window"
(Assignee)

Comment 10

17 years ago
Created attachment 44421 [details] [diff] [review]
Alternate fix: Use "Existing Navigator window" only for Composer

Comment 11

17 years ago
I like the Composer-only fix. sr=blake
(Assignee)

Comment 12

17 years ago
Created attachment 44437 [details] [diff] [review]
Updated alternate fix: Must use "setAttribute" to change menuitem label

Comment 13

17 years ago
Right. Missed that.  Actually, .label should work for menuitems, but that's a
separate bug.

Comment 14

17 years ago
r=brade on the most recent patch (provided the *.properties file is also checked
in with it)
(Assignee)

Comment 15

17 years ago
checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Keywords: patch, review
Resolution: --- → FIXED
Whiteboard: FIXINHAND need r=, sr=

Comment 16

17 years ago
Verified on 8-30 build
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.