558485 - Don't show the page's URL in the status bar for links in the plugin crash sadface placeholder

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect)

Description

[(no longer active)]

Tomer pointed out to me today that for some reason, we show the page's URL in the status bar when hovering on the links inside the plugin crash placeholder.  Considering the fact that the UI is part of the chrome, we shouldn't probably do that.

This _might_ make some spoofing attempts possible, I'm not so sure.  Filing it in the security group for now.

Comment 1

[Benjamin Smedberg]

sg:low at worst, not worth keeping private.

Comment 2

[Brandon Sterne (:bsterne)]

Code freeze for 1.9.2.4 is midnight tonight.  We probably can't block the release for this but marking as wanted.

Comment 3

[(no longer active)]

Attachment: Patch (v1)

Use html:span's instead of html:a's, and style them as links.

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 438564 [details] [diff] [review]
Patch (v1)

>-html|a {
>+html|a, .pleaseSubmitLink, .reloadLink {
>   color: white;
>+  text-decoration: underline;
>+}

Are there other 'a' elements that are matched here?
color:white is probably not needed for .pleaseSubmitLink and .reloadLink if they are spans.

>+.pleaseSubmitLink:hover, .reloadLink:hover {
>+  cursor: pointer;

:hover is superfluous

Comment 5

[(no longer active)]

Attachment: Patch (v2)

(In reply to comment #4)
> (From update of attachment 438564 [details] [diff] [review])
> >-html|a {
> >+html|a, .pleaseSubmitLink, .reloadLink {
> >   color: white;
> >+  text-decoration: underline;
> >+}
> 
> Are there other 'a' elements that are matched here?
> color:white is probably not needed for .pleaseSubmitLink and .reloadLink if
> they are spans.

For some reason, I thought that this CSS is included elsewhere as well, but I was wrong.

> >+.pleaseSubmitLink:hover, .reloadLink:hover {
> >+  cursor: pointer;
> 
> :hover is superfluous

You're right.  Here's the updated patch.

Comment 6

[Justin Dolske [:Dolske]]

Also:

Add role="link" to the 2 new spans.

Remove the "// XXX can we make the link target actually be blank?" from browser.js. The "<!-- link href set at runtime -->" comments can be removed from the XBL too (since it's not longer a confusing <a> without an href).

And in browser.js's addLinkClickCallback(), the "evt.preventDefault();" can be removed (since the purpose was to avoid triggering the actual link).

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 438573 [details] [diff] [review]
Patch (v2)

@namespace html is now unused.

>+.pleaseSubmitLink, .reloadLink {

nit: line break after the comma

Are the links still focusable? Probably not, so r-.

Comment 8

[Dão Gottwald [:dao]]

Also, I don't really see the spoofing risk...

Comment 9

[(no longer active)]

Attachment: WIP

This should make the link focusable, but doesn't.

Comment 10

[(no longer active)]

Unassigning from myself.  Someone else can probably get the patch and make it work.

Comment 11

[Johnny Stenback (:jst)]

Not blocking the release on this.

Comment 12

[Benjamin Smedberg]

Resolving old bugs which are likely not relevant any more, since NPAPI plugins are deprecated.