Bookmarking image map links should get something for bookmark name

VERIFIED FIXED in mozilla0.9.8

Status

SeaMonkey
Bookmarks & History
P5
normal
VERIFIED FIXED
17 years ago
14 years ago

People

(Reporter: Heikki Toivonen (remove -bugzilla when emailing directly), Assigned: Heikki Toivonen (remove -bugzilla when emailing directly))

Tracking

({top100})

Trunk
mozilla0.9.8
top100

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixinhand], URL)

Attachments

(1 attachment, 4 obsolete attachments)

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>
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
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Er... this should be trivial, and this is affects top100 sites...
Keywords: top100
Paul Chen is now taking Bookmarks bugs. For your convenience, you can filter 
email notifications caused by this by searching for 'ilikegoats'.

Assignee: ben → pchen
Status: ASSIGNED → NEW

Comment 4

17 years ago
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"
Priority: -- → P5
Taking, I have a fix.
Assignee: pchen → heikki
Severity: enhancement → normal
Target Milestone: Future → mozilla0.9.7
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?
Created attachment 60724 [details] [diff] [review]
In addition to title, try alt attribute
Created attachment 60729 [details] [diff] [review]
And finally if alt is also empty just get the URL

Comment 9

17 years ago
Will this patch also fix bug 91448?
Whiteboard: [fixinhand]
timeless, r=? blake, sr=?

Comment 12

17 years ago
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.
Attachment #60729 - Flags: review+
Target Milestone: mozilla0.9.7 → mozilla0.9.8
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.
Attachment #60721 - Attachment is obsolete: true
Attachment #60724 - Attachment is obsolete: true
Attachment #60729 - Attachment is obsolete: true
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.
Created attachment 62004 [details] [diff] [review]
Correct patch

Sorry, the previous patch was bogus, this is the correct one.
Attachment #61990 - Attachment is obsolete: true

Comment 16

17 years ago
Comment on attachment 62004 [details] [diff] [review]
Correct patch

sr=blake

Why the deep nesting?
Attachment #62004 - Flags: superreview+
Attachment #62004 - Flags: review+
If you mean deap nesting vs. multiple returns, I usually prefer for a function
to have just one return statement.
Fixed.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
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.]
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.