306937 - "Bookmark This Link..." context menu appends bogus whitespace characters to the bookmark name

Status: NEW

Description

[shutdown]

Try to "Bookmark This Link..." on the "Component" link in this bug page.
It opens a new "Add Bookmark" dialog with the Name field filled by
the string "Co m ponent" instead of the expected string "Component".

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b4) Gecko/20050901 Firefox/1.0+

Comment 1

[Adam Guthrie]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20050902 Firefox/1.6a1
ID:2005090216

Confirming. This probably has something to do with how we parse HTML code within
a link's title.

<a href="describecomponents.cgi?product=Firefox">
            Co<u>m</u>ponent</a>:

Comment 2

[Phil Ringnalda (:philor)]

"parse HTML" is putting it pretty strongly. You're looking at changing
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/utilityOverlay.js&rev=1.32&mark=327&#318
to conditionally add a space based on whether or not node.parentNode or
node.previousSibling would produce some whitespace, so that
<a>bar<br>baz<div>quux</div></a> would still have spaces.

Comment 3

[shutdown]

nsIDocumentEncoder can solve such complex HTML-formatting issues very well.
(it can handle normal texts, IMG alts, SPANs with display:block, etc...)
We need a scriptable interface to use it. I suggest adding a method
toStringWithFormat() to nsIDOMNSRange, like nsISelectionPrivate.

http://lxr.mozilla.org/seamonkey/source/dom/public/idl/range/nsIDOMNSRange.idl#43
http://lxr.mozilla.org/seamonkey/source/content/base/public/nsISelectionPrivate.idl#75
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsSelection.cpp#1511

Comment 4

[Marco Bonardo [::mak]]

still valid 
Mozilla/5.0 (Windows; U; Windows NT 6.0; it; rv:1.9.0.4) Gecko/2008102920 Firefox/3.0.4

Comment 5

[Nickolay_Ponomarev]

Copying a comment from bug 274486 comment 8:

(In reply to Phil Ringnalda (:philor) from comment #8)
> Actually, not smarter, just more persistent. Using a documentencoder's
> fairly easy, something like
> 
> +  const kEncoderCIDbase = "@mozilla.org/layout/documentEncoder;1?type=";
> +  var doc = root.ownerDocument;
> +
> +  if ((kEncoderCIDbase + doc.contentType) in Components.classes) {
> +    const nsIDocumentEncoder = Components.interfaces.nsIDocumentEncoder;
> +    var encoder = Components.classes[kEncoderCIDbase + doc.contentType]
> +                            .createInstance(nsIDocumentEncoder);
> +    var flags = encoder.OutputNoScriptContent |
> encoder.OutputNoFramesContent;
> +    encoder.init(doc, "text/plain", flags);
> +    encoder.setNode(root);
> +    return encoder.encodeToString().replace(/^\s+|\s+$/g, "")
> +                                   .replace(/\s+/g, " ");
> 
> (with an else, and maybe a try, too), but since that involves using two
> essentially untested things, documentencoder and plaintextserializer,
> someone's going to be in for a whole lot of test writing to land it.

An updated link to gatherTextUnder: http://hg.mozilla.org/mozilla-central/annotate/fd06332733e5/browser/base/content/utilityOverlay.js#l394

Comment 7

[Jesse Ruderman]

Also affects Tweetdeck. Right-clicking a mentioned username shows e.g. "Search Google for @ jruderman", with an extra space after the "@", because they're in different spans.