89710 - Bookmark this Link (context menu) doesn't work for links enclosed by <FONT> tags

Status: VERIFIED FIXED

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

Description

[Claudius Gayle]

I found this bug while testing bug 68985.

***Overview Description: 
  The context menu command 'Bookmark this link' isn't firing at
all for some subset of links.

***Steps to Reproduce: 
1) Surf to www.yahoo.com
2) Right-click any bold link
3) Select 'Bookmark this Link'

***Actual Results: 
   nada. nothing.

***Expected Results: 

  Thanks to bug 68985 being fixed an add bookmark dialog should appear.

***Build Date & Platform Info: 

  I've seen this broken on all platforms with both Branch AND trunk
builds from 20010706. I've seen this working as recently as a 2001070503
branch build on Win98. That tells me this is a recent regression that
was probably caused by some checkin that went into the bracnh within the
last day or so.

***Additional Information:

  The repro steps point to links with a bold styling and indeed every
link on that page that is bold exhibits the broken behavior whereas I
could not find a broken non-bold link.
  However, on the Netscape homepage it is the links that do the
mouseover underline thing that have this broken behavior.

Comment 1

[Paul Chen]

nav triage team:

On a mac debug commercial build, console prints out:

JavaScript error: 
chrome://communicator/content/nsContextMenu.js line 618:
node.localName.ToUpperCase is not a function

last person to touch that line was Heikki in the latest version of
nsContextMenu.js, version 1.29. Reassigning to heikki

Comment 2

[doctor__j]

Attachment: Testcase

Comment 3

[doctor__j]

Changing summary.

Comment 4

[Claudius Gayle]

doctor_j, that's not correct. The <font> tag was my first suspect as well but that's not the case.
On yahoo's site you can see links that aren't enclosed by the font tag (just bold) and still
exhibit the buggy behavior. That's what caused me to suspect the 'bold' tag. As I have
already stated however that tag alone is not the culprit. Both tags are 'sufficient' conditions
but each taken alone is not a 'necessary' condition.
What we are proabably looking for is some class of tags that includes both of these as a
superset.

Comment 5

[doctor__j]

Attachment: Another testcase

Comment 6

[doctor__j]

A wild guess: The bug appears whenever the link is enclosed by some "font style"
tags, like <B>, <I>, <FONT>, those sorts of "style-related-tags".

Comment 7

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

Argh, typo! I'll attach the fix.

Comment 8

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

Attachment: Fix (upper case function name to lower case)

Comment 9

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

Folks, can you give r & sr? My fix to bug 86894 introduced a typo which caused
this bug.

Comment 10

[Blake Ross]

sr=blake

Comment 11

[harishd]

r=harishd

Comment 12

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

This affects top100 sites (it is common to style links), inluding
http://home.netscape.com main page. 

Since QA found this bug the next day it was introduced, I think there are people
who frequently use this feature.

The fix is changing 1 character to upper case in a JavaScript file.

Comment 13

[Blake Ross]

Yes, nearly all of the links on netscape.com (and on many other sites) are
affected by this. The change is obvious, and the benefit is big. Heikki, can you
e-mail [e-mail] and ask that they consider this?

Comment 14

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

Yeah, I should have mentioned, but I did email pdt2 already asking for
permission to check this to the branch.

Comment 15

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

Fixed on the trunk. Leaving still open in the hopes that we'll get this on the
branch.

Comment 16

[selmer (gone)]

PDT+, check in right away and we'll pick this up in a respin.

Comment 17

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

Fixed also on the branch. Marking fixed.

Comment 18

[Henrik Gemal]

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

Comment 19

[Claudius Gayle]

VERIFIED Fixed with 2001072006 branch build. adding vtrunk keyword if that's still how
that works.

Comment 20

[Claudius Gayle]

Hmmm, I'm not sure whether to reopen this or create a new bug. The fix is incomplete...
I spoke too hastily and we're still broken on the Mac. We now create a disiabled menuitem
"Blank menu item" where the title of the bookmark should be. This menuitem is useless
as it is grayed out (disabled). On windows we are correctly grabbing the text between the
<a></a> tags.
The mac is no worse than it was before this fix so no harm done there - but we still have a
very recent regression.

I'm going to reopen for now. PDT or someone can decide how to proceed.

Comment 21

[selmer (gone)]

I'd be interested in seeing a fix.

Comment 22

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

I am not seing this on today's trunk Mozilla build on the Mac. Could you please
give me exact instructions on how to reproduce the bug on the Mac (the exact
page and link included). Or is this branch specific?

Comment 23

[selmer (gone)]

What's the scoop on this bug for the branch?  We're about to ship with this bug
unless someone *does* something...

Comment 24

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

I have not been able to install a late branch build, the installer hangs my Mac...

Comment 25

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

I am building Mac branch bits now, will get to test in a couple of hours...

Comment 26

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

Mac Mozilla branch also works fine for me...

Comment 27

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

Ok, spoke with Claudius on the phone, and we found at least one set of links
which produces blank name. On http://home.netscape.com the links on the left:
"Autos", "Browser Central" etc. cause this problem on all platforms. On Windows
we have a small icon in front of the empty name so you can still access that
bookmark. On Mac there is no icon so you can't do that.

Now that I know where this is happening I can look at it...

Comment 28

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

Ok, the links on http://home.netscape.com are actually image maps and not text.
I checked a build from 6/11 and they do not work there either. My guess is they
have never worked.

I opened bug 92172 for the enhancement.

Marking this one fixed.

Comment 29

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

Damnit, I want an enhancement to Bugzilla so that when I type "fixed" it will
mark the bug fixed. Stupid Bugzilla, isn't it obvious I meant to mark this fixed...

Comment 30

[Claudius Gayle]

OK, so with 20010724 Branch and Trunk builds this bug is VERIFIED Fixed.
marking as such.