74019 - link with bad protocol causes full context menu

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Cesar Eduardo Barros]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.4.2 i686; en-US; 0.8.1) Gecko/20010314
BuildID:    2001031408

The link to adcops.com in the page is broken (htt:// instead of http://).
Right-clicking on it shows a context menu with every possible option, including
an (&A) option which makes no real sense (& meaning underline here). The copy
link location option (which was what I wanted in first place) did absolutely
nothing.

Reproducible: Always
Steps to Reproduce:
1. Open the page
2. Click in the broken AdCops.com link with the right button

Actual Results:  A full context menu which doesn't work as it should

Expected Results:  A normal context menu with at least "copy link location" working.

The following shows up on console after each right-click on the link:

JavaScript error: 
 line 0: uncaught exception: [Exception... "<no message>"  code: "-2142568430"
nsresult: "0x804b0012 (<unknown>)"  location:
"chrome://communicator/content/nsContextMenu.js Line: 347"]

JavaScript error: 
chrome://cookie/content/cookieContextOverlay.xul line 88: contextMenu has no
properties

JavaScript error: 
 line 0: contextMenu has no properties

Comment 1

[timeless]

great :(

Comment 2

[sairuh (rarely reading bugmail)]

ack, badness. confirming. will add testcase soon, since the original url [
http://www.msnbc.com/news/550567.asp?cp1=1 ] now has that broken link fixed.

Comment 3

[sairuh (rarely reading bugmail)]

Attachment: page w/bad htt:// link

Comment 4

[sairuh (rarely reading bugmail)]

happens on winNT and mac, too.

who should get this? steve, is this yours perchance?

Comment 5

[sairuh (rarely reading bugmail)]

another observation: if there are other links on the page, and you bring up a
context menu on one of them which has a valid protocol, then this bug doesn't
occur. interesting.

updating summary. will attach another testcase...

Comment 6

[sairuh (rarely reading bugmail)]

Attachment: updated testcase

Comment 7

[Paul Chen]

nav pretriage:

Although annoying, I don't think we have time to fix this one for beta1, suggest 
marking nsbeta1-

Comment 8

[Viswanath Ramachandran]

nav triage : this is not a beta stopper. 

Comment 9

[Bradley Baetz]

OK, I've got a patch. The problem was that nsHTMLAnchorElement::GetProtocol
calls NS_NewURI, instead of doing the parsing itsself. So if the protocol is
invalid, that fails, and we get the error code propogated back to JS.

And for some reason (and I've seen this on several bugs, as well as when I've
made typos in this file), JS errors when bringing up the context menu cause all
possible entries to be shown. That may be by design (its better than showing
nothing) - I really don't know.

Since .protocol doesn't appear in the DOM2 docs, I have no idea what the correct
behaviour should be.

The patch I'll attach just adds a try { ... } catch (e) { } arround the
appropriate block, and returns false from isSaveableLink, since that link would
not be able to be saved.

Comment 10

[Bradley Baetz]

Attachment: patch

Comment 11

[Peter "jag" Annema]

Looks okay, but please reindent the block you're wrapping. r=jag if you do that.

Comment 12

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

sr=ben

Comment 13

[André Dahlqvist]

Do we want this one for 0.9?

Comment 14

[Bradley Baetz]

This won't make it into 0.9 - its not a critical fix. I'll check it into the
truck after the tree branches (tomorrow?)

Reassigning to me and changing other fields so that it comes up on my queries

Comment 15

[Bradley Baetz]

Fix checked in.

Comment 16

[sairuh (rarely reading bugmail)]

vrfy fixed using opt comm bits:

winnt - 2001.05.17.10
linux and mac - 2001.05.17.08