92172 - Bookmarking image map links should get something for bookmark name

Status: VERIFIED FIXED

Description

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This grew out from bug 89710. It turns out if you bookmark an image map link the
name property is blank. This will make it impossible to use that bookmark on the
Mac. On Windows at least you get an icon to show the blank spot so you can edit
the name (the bookmark itself still works).

I don't know what would be the best string to put in name. Maybe get title if
the attribute is present, or alt or something like that.

Here is sample markup from Netscape.com:

<map NAME="HP_LEFTNAV">
<area ALT="Autos" COORDS="0,2,114,17"
HREF="http://info.netscape.com/fwd/hop07d1/http://webcenters.netscape.com/webcenters/autos/netscape/home.adp"
SHAPE=RECT>

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This bug is about when you right-click and select Bookmark This Link from the
context menu when you are hovering over an image map link, like the "Autos",
"Browser Central" etc. links on http://home.netscape.com

Comment 2

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Er... this should be trivial, and this is affects top100 sites...

Comment 3

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

Paul Chen is now taking Bookmarks bugs. For your convenience, you can filter 
email notifications caused by this by searching for 'ilikegoats'.

Comment 4

[Paul Chen]

Mass move Ben's bugs dumped on me marked future with p5 to get off my untriaged
radar. You can filter out this email by looking for "ironstomachaussie"

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Taking, I have a fix.

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Created attachment 60721 [details] [diff] [review]
Proposed fix

The idea here is that in content menu's linkText() function, if we got nothing
as text from the contents of the link, return the link elements title
attribute. This is what IE seems to be doing with AREA elements, for example.

Reviews?

Comment 7

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Created attachment 60724 [details] [diff] [review]
In addition to title, try alt attribute

Comment 8

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Created attachment 60729 [details] [diff] [review]
And finally if alt is also empty just get the URL

Comment 9

[Chris Lyon]

Will this patch also fix bug 91448?

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This does not fix  bug 91448.

Comment 11

[Heikki Toivonen (remove -bugzilla when emailing directly)]

timeless, r=? blake, sr=?

Comment 12

[timeless]

Comment on attachment 60729 [details] [diff] [review]
And finally if alt is also empty just get the URL

we talked about this a bit on irc. !text.match(/\w/) would be prefered and you
can have my r= to change each text condition to that too, 
The reason for this change is that alt="   "  (or any of the other things that
can result in whitespace) are not useful to the user whereas following the
fallback codepath is.

Comment 13

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Created attachment 61990 [details] [diff] [review]
Adderssing timeless' concerns

This is basically the same patch, but instead of just checking for empty string
we check if it consists only of whitespace. I noticed we needed this check with
XLinks as well.

Comment 14

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Oh, and \w did not seem to work, Axel Hecht suggested I use \S, which is what I
did and it seems to work ok.

Comment 15

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Created attachment 62004 [details] [diff] [review]
Correct patch

Sorry, the previous patch was bogus, this is the correct one.

Comment 16

[Blake Ross]

Comment on attachment 62004 [details] [diff] [review]
Correct patch

sr=blake

Why the deep nesting?

Comment 17

[Heikki Toivonen (remove -bugzilla when emailing directly)]

If you mean deap nesting vs. multiple returns, I usually prefer for a function
to have just one return statement.

Comment 18

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fixed.

Comment 19

[sairuh (rarely reading bugmail)]

mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]