354251 - "Open in Tabs" string missing from History menu under "Recently Closed Tabs" (if you disable places)

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[(not reading, please use [e-mail] instead)]

"Open in Tabs" string missing from History menu under "Recently Closed Tabs" (if you disable places)

http://litmus.mozilla.org/show_test.cgi?id=2468

from the console:

JavaScript error: chrome://browser/content/browser.js, line 6610: strings has no properties

here comes a screen shot

Comment 1

[(not reading, please use [e-mail] instead)]

Attachment: screen shot

Comment 2

[(not reading, please use [e-mail] instead)]

here's the cause, I think:

http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser.js#6887

Comment 3

[(not reading, please use [e-mail] instead)]

Attachment: patch

note, these strings are duplicated.  the still live in places.properties, for use by http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser-places.js#420

Comment 4

[(not reading, please use [e-mail] instead)]

in addition to the menu not having a label, the menu item won't work as we'll throw that error

Comment 5

[Dietrich Ayala (:dietrich)]

(In reply to comment #3)
> Created an attachment (id=240099) [edit]
> patch
> 
> note, these strings are duplicated.  the still live in places.properties, for
> use by
> http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser-places.js#420
> 

Instead of duplicating the strings, would it be possible to remove the duplicate strings from places.properties, and use gNavigatorBundle with the browser.properties string in both locations? These are the only consumers of the property, so it'd be great not to duplicate.

http://lxr.mozilla.org/mozilla/search?string=menuOpenInTabs.label

Comment 6

[(not reading, please use [e-mail] instead)]

should be doable, since:

/browser/base/content/browser.js, line 6769 -- #include browser-places.js

so we should have access to browser.properties (via bundle_browser).

I'll investigate that (for --enabled-places, to make sure I don't break that.)

Comment 7

[(not reading, please use [e-mail] instead)]

Attachment: better patch

here's a better patch (thanks to dietrich for the suggestion) to move (instead of duplicating) the strings.

I've tested on trunk both with places (and without, which is now the default.)

Comment 8

[(not reading, please use [e-mail] instead)]

fixed