Closed
Bug 593646
Opened 14 years ago
Closed 14 years ago
rename 'Open in a New Window' to 'Move to New Window' and move it to a more sensible location
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 4.0b7
People
(Reporter: fryn, Assigned: fryn)
References
Details
(Whiteboard: [strings])
Attachments
(1 file, 1 obsolete file)
Current tab context menu: ... ------------------------- Open in a New Window Make into App Tab Move to Group > ------------------------- ... Should be the following for clear language and strictly increasing level of movement: ... ----------------------- Make into App Tab Move to Group > Move to New Window ----------------------- ... We should remove the article "a" for consistency, since we don't use it in items, such as "Open Link in New Window". We ought to get this in by beta6 for string freeze!
Attachment #472218 -
Flags: ui-review?(faaborg)
Attachment #472218 -
Flags: review?(dao)
Comment 1•14 years ago
|
||
Comment on attachment 472218 [details] [diff] [review] patch Yeah, looks like we never updated strings when we got tab tear off working without a refresh, thanks for catchig this.
Attachment #472218 -
Flags: ui-review?(faaborg) → ui-review+
Comment 2•14 years ago
|
||
There has never been a refresh, this feature explicitly waited for bug 113934. "Move to a New Window" was considered in bug 482450, too.
Comment 3•14 years ago
|
||
Comment on attachment 472218 [details] [diff] [review] patch r- as it stands, needs more careful consideration.
Attachment #472218 -
Flags: review?(dao) → review-
Comment 4•14 years ago
|
||
however, bug 559431 suggest changing it to "detach tab"
Assignee | ||
Comment 5•14 years ago
|
||
(In reply to comment #3) > Comment on attachment 472218 [details] [diff] [review] > patch > > r- as it stands, needs more careful consideration. If I understand correctly, "review" refers to code review, not UI review.
Comment 6•14 years ago
|
||
Review means sanity checking in various regards. There was obviously a wrong assumption in comment 1, so I could as well have canceled the ui-review+. Either way this isn't ready to land. I'll be happy to re-review this once it's sorted if there was a good reason for preferring "Open in a New Window" over "Move to a New Window" in bug 482450.
Comment 7•14 years ago
|
||
+1 for Move. It doesn't open the tab, it moves it. Not sure how much more reasoning is needed. :)
Comment 8•14 years ago
|
||
I agree with Frank's proposed changes. "Open" implies a reload - it's the first step of going to a web destination. "Move" implies relocation but not reload. As opening in a new window preserves DOM state and (mostly) doesn't reload, move is a more appropriate term than open. Removing article "a" is good too - we generally don't use articles in strings because they are not needed for clarity
Comment 9•14 years ago
|
||
Limi: not quite right; we should look through the old bugs and understand the context, looking before leaping. Having done that, I don't think there was an explicit reason. Additionally, now that we have "Move to Group" it makes sense to also have "Move to Window" instead of "Open in New Window". Finally, I think "Open in New Window" may lead people to believe it will behave more like "duplicate" than "move". Thanks for checking, Dao (although cancelling the ui-r wouldn't be apropos, more likely re-requesting along with your concerns!)
Assignee | ||
Comment 11•14 years ago
|
||
Comment on attachment 472218 [details] [diff] [review] patch Thanks for the input, beltzner, boriss, and limi. Re-requesting review.
Attachment #472218 -
Flags: review- → review?
Updated•14 years ago
|
Attachment #472218 -
Flags: review? → review+
Assignee | ||
Comment 12•14 years ago
|
||
Comment on attachment 472218 [details] [diff] [review] patch Dao, while I may have protested at first, your sanity check resulted an improved process, and we are better for it. Thanks for the review.
Attachment #472218 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #472218 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Updated•14 years ago
|
Whiteboard: [strings]
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Comment 13•14 years ago
|
||
patching file browser/base/content/browser.xul Hunk #1 FAILED at 110 1 out of 1 hunks FAILED -- saving rejects to file browser/base/content/browser.xul.rej patching file browser/locales/en-US/chrome/browser/browser.dtd Hunk #1 FAILED at 12 1 out of 1 hunks FAILED -- saving rejects to file browser/locales/en-US/chrome/browser/browser.dtd.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh attachment.cgi?id=472218
Keywords: checkin-needed
Comment 14•14 years ago
|
||
I'll be landing tomorrow, so will make sure this gets in as a ridealong if it hasn't already. Frank if you can update the patch (make sure it has user info + commit comment) http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed Also add this bug to the ride along section of the landing queue, since people are looking at it. https://wiki.mozilla.org/LandingQueue
blocking2.0: ? → ---
Assignee | ||
Comment 15•14 years ago
|
||
zpao, thanks for the link and help.
Attachment #472218 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 16•14 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/16071f59d247
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b7
Comment 17•14 years ago
|
||
Verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100918 Firefox/4.0b7pre
Status: RESOLVED → VERIFIED
Comment 18•14 years ago
|
||
Litmus test has been updated: https://litmus.mozilla.org/show_test.cgi?id=10010
Flags: in-litmus+
You need to log in
before you can comment on or make changes to this bug.
Description
•