Closed Bug 92172 Opened 20 years ago Closed 19 years ago

Bookmarking image map links should get something for bookmark name

Categories

(SeaMonkey :: Bookmarks & History, defect, P5)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.8

People

(Reporter: hjtoi-bugzilla, Assigned: hjtoi-bugzilla)

References

()

Details

(Keywords: top100, Whiteboard: [fixinhand])

Attachments

(1 file, 4 obsolete files)

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
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
Attached patch Proposed fix (obsolete) — Splinter Review
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?
Will this patch also fix bug 91448?
Whiteboard: [fixinhand]
timeless, r=? blake, sr=?
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
Attached patch Adderssing timeless' concerns (obsolete) — Splinter Review
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.
Attached patch Correct patchSplinter Review
Sorry, the previous patch was bogus, this is the correct one.
Attachment #61990 - Attachment is obsolete: true
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
Closed: 19 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.