164155 - Title beginning with hyphen invisible in bookmarks list (shown as separator)

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Niklas Dougherty]

Bookmarks with a title beginning with a hyphen will not show in the bookmarks
menu. It will leave an empty hole in the list. It does not happen with other
kinds of dashes (endash, emdash and so on), and it doesn't matter what comes
after the hyphen.

It is not possible to selct such a bookmark in the list.

Steps to reproduce:

1. Go to http://www.ostas.lu.se/kurser/KIN/KIN213/default.html
2. Bookmark page
3. Go to Bookmarks and see what happened
4. Try to select the new bookmark
5. Add another (regular) bookmark
6. Go to Bookmarks again and see effect
7. Manage Bookmarks to really see it's there
8. Create a bookmark with a title beginning with a hyphen to confirm once and
for all

MOX 2002-08-21-09

Comment 1

[Niklas Dougherty]

Attachment: Screen dump

This shows the Bookmarks menu side to side with the Manage Bookmarks list.

Comment 2

[Niklas Dougherty]

On dividers in bookmarks: IE implements a href type called "Netscape bookmark"
in its bookmark file to signify a divider, and it consists of a *single* hyphen.
IE renders bookmarks having a title beginning with a hyphen correctly.

Comment 3

[Greg K.]

Confirmed using Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.1)
Gecko/20020818.

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Mass reassign of my non-Firefox bugs to [e-mail]

Comment 5

[HARUNAGA Hirotoshi]

The URL doesn't have title beginning with a hyphen now.
Changing the URL.

From long time ago, it's shown as a separator on Mac.

Bug 238549 on Firefox should be a dup of this.

Comment 6

[Ben Fowler]

Attachment: Backtrace shwoing messages evinced bookmarking this URL

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a) Gecko/20040425
Firefox/0.8.0+

This bug is still in

Comment 7

[Bruce Davidson]

*** Bug 238549 has been marked as a duplicate of this bug. ***

Comment 8

[Bruce Davidson]

As I noted on the corresponding Firefox bug, this actually has nothing to do
with bookmarks - its a problem with how MacOS treats menu labels. Changing the
component to reflect this.

Any menu item that starts with a hyphen will appear as a separator. (See
http://developer.apple.com/tools/mpw-tools/commandref/appc_menu.html). We can
work around this by adding a zero width space (‌) to the start of the menu
item before the hyphen. I've checked that this works when editing the DTD for a
menu. I'm now working on a patch to nsMenuX that will automatically prepend a
zero width space to any menu item that starts with a hyphen.

Comment 9

[Bruce Davidson]

Attachment: Patch to "escape" leading hyphens in Mac menu entries

Patch that makes sure any menu item (whether a bookmark or a normal menu item)
with a leading hyphen has the hyphen "escaped" with a zero width space. Tested
on both Firefox and Mozilla for both bookmarks and other menu items.

Comment 10

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 148137 [details] [diff] [review]
Patch to "escape" leading hyphens in Mac menu entries

looks ok. does that work correctly on 10.1 as well? Just making sure we're not
relying on something apple fixed/implemented in the carbon menu manager post
10.1.

Comment 11

[Bruce Davidson]

Cheers Mike. If anyone here can apply this patch on a 10.1 system and comment
back here that would be helpful!

Comment 12

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 148137 [details] [diff] [review]
Patch to "escape" leading hyphens in Mac menu entries

r=pink

i think bryner might have a 10.1 partition at MF. if he's going to sr, he might
be able to give it a quick test.

Comment 13

[Mike Pinkerton (not reading bugmail)]

oh, 2 nits about the patch:

- don't use braces for a single line if
- 2-space indent is the mozilla standard, the patch uses 4-space indent

Comment 14

[Bruce Davidson]

Attachment: Upated patch to address nits raised by Pinkerton

Comment 15

[Bruce Davidson]

Comment on attachment 148226 [details] [diff] [review]
Upated patch to address nits raised by Pinkerton

Updated patch. Mike, can you transfer your r= to this new patch - which
addresses the two nits you raised.

Comment 16

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 148226 [details] [diff] [review]
Upated patch to address nits raised by Pinkerton

r=pink

Comment 17

[Bruce Davidson]

I tried my private build of Firefox (built on my 10.3.4 box) on my parents' 10.1
box. Unfortunately it didn't load up (something about a missing .dylib - do you
need particular build options to do a build that works on 10.1?). Doesn't seem
to be related to this fix thought.

Mike, do you still want to wait for confirmation that this works on 10.1 (either
from another attempt by myself, or bryner if he still has a 10.1 box at MF), or
should we just land this to make sure it gets in for FF1.0, and see if anyone
actually reports at 10.1 bug. I can't see that the patch could do anything too
nasty on 10.1 (I guess you might get a corrupt first character for such a
bookmark rather than a separator). Thoughts?

Comment 18

[Mike Pinkerton (not reading bugmail)]

yeah, you need to ensure you build against the 10.2 sdk for your private build
to run on 10.1. camino requires this as part of the build process, i guess FF
doesn't. 

regardless, i think we're ok with landing this. if not, someone will complain ;)

Comment 19

[Mike Pinkerton (not reading bugmail)]

landed.