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

VERIFIED FIXED in mozilla0.9.3

Status

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

People

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

Tracking

({regression, top100})

Trunk
mozilla0.9.3
regression, top100

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PDT+ (some links still broken), URL)

Attachments

(3 attachments)

(Reporter)

Description

17 years ago
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.

Updated

17 years ago
Keywords: nsbeta1

Comment 1

17 years ago
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
Assignee: ben → heikki

Comment 2

17 years ago
Created attachment 41938 [details]
Testcase

Comment 3

17 years ago
Changing summary.
Summary: Bookmark this Link(context menu) doesn't work for some kinds of links → Bookmark this Link (context menu) doesn't work for links enclosed by <FONT> tags
(Reporter)

Comment 4

17 years ago
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

17 years ago
Created attachment 42272 [details]
Another testcase

Comment 6

17 years ago
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".
Argh, typo! I'll attach the fix.
Status: NEW → ASSIGNED
Keywords: regression
Priority: -- → P2
Whiteboard: [fixinhand]
Target Milestone: --- → mozilla0.9.3
Created attachment 42501 [details] [diff] [review]
Fix (upper case function name to lower case)
Folks, can you give r & sr? My fix to bug 86894 introduced a typo which caused
this bug.

Comment 10

17 years ago
sr=blake

Comment 11

17 years ago
r=harishd
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.
Keywords: top100

Comment 13

17 years ago
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 pdt2@netscape.com and ask that they consider this?
Yeah, I should have mentioned, but I did email pdt2 already asking for
permission to check this to the branch.
Fixed on the trunk. Leaving still open in the hopes that we'll get this on the
branch.
Whiteboard: [fixinhand] → [fixed on trunk]

Comment 16

17 years ago
PDT+, check in right away and we'll pick this up in a respin.
Whiteboard: [fixed on trunk] → PDT+ [fixed on trunk]
Fixed also on the branch. Marking fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Whiteboard: PDT+ [fixed on trunk] → PDT+

Comment 18

17 years ago
*** Bug 91280 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 19

17 years ago
VERIFIED Fixed with 2001072006 branch build. adding vtrunk keyword if that's still how
that works.
(Reporter)

Comment 20

17 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 21

17 years ago
I'd be interested in seeing a fix.
Whiteboard: PDT+ → PDT+ (Mac still broken)
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

17 years ago
What's the scoop on this bug for the branch?  We're about to ship with this bug
unless someone *does* something...
I have not been able to install a late branch build, the installer hangs my Mac...
I am building Mac branch bits now, will get to test in a couple of hours...
Mac Mozilla branch also works fine for me...
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...
Whiteboard: PDT+ (Mac still broken) → PDT+ (some links still broken)
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.
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...
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 30

17 years ago
OK, so with 20010724 Branch and Trunk builds this bug is VERIFIED Fixed.
marking as such.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.