1438660 - Status panel shows original @href value after it's modified by mouseenter listener

Status: RESOLVED DUPLICATE of bug 494876

(Core :: DOM: Core & HTML, defect, P3)

Description

[Victor Engmark]

Attachment: index.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180207210535

Steps to reproduce:

1. Open the attached index.html
2. Move the mouse so that it hovers over the link *once*


Actual results:

The status panel displays "https://example.org".


Expected results:

The status panel should display "http://example.org".

Comment 1

[Victor Engmark]

Chromium does the right thing here: it displays "http://example.org".

Also, this is similar to, but distinct from, bug #229050, because the correct @href is right there in the DOM. The link colour also changes to purple on hover (because I've only visited the http:// URL), so it seems that the status panel is simply rendered too early.

Comment 2

[u554753]

Reproducible with the following specs:
Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0

Comment 3

[Marcos Caceres [:marcosc]]

This is a duplicate of a bug I can't find :( This is basically handled by Firefox's UI, that is supposed to watch for changes to hyperlinks and update this UI component. However, it's not a DOM issue, I don't think.

Comment 4

[:Gijs (he/him)]

(In reply to Marcos Caceres [:marcosc] from comment #3)
> This is a duplicate of a bug I can't find :( This is basically handled by
> Firefox's UI, that is supposed to watch for changes to hyperlinks and update
> this UI component. However, it's not a DOM issue, I don't think.

Funnily enough, I think this *is* a DOM issue. The only thing frontend does for this panel is expose a 'setStatus' API that is called by docshell, which is called by DOM:

https://dxr.mozilla.org/mozilla-central/rev/b906009d875d1f5d29b0d1252cdb43a9b1a5889c/dom/base/Element.cpp#3271

The setstatus/setstatuswithcontext stuff is forwarded over IPC by the dom/ipc/TabChild and TabParent code. At that point (ie in the parent process), there's no element anymore for frontend to use to listen to changes. So if we wanted to fix this, it'd need to be fixed in DOM code so it'd dispatch another setstatus message if the href of the last-status'd element changes, or if the element is removed. We could do that by making the toplevel docshell track the last element it was told to use for a status (probably with a weakref?), and have DOM try to tell docshell if it went away or its href changed. Or something.

Seems pretty expensive/hairy though, we'd be getting and releasing refs (and/or mutationobservers...) to that element as you mouse over the document...


Taking a step back, I don't think there's much of a point investing time/effort in fixing this bug... It's impossible for the status element to always be accurate, it's a losing battle. The href can change on mousedown, or the website could just be calling .preventDefault() on the link click (stopping the navigation) and then setting location.href to an arbitrary string.

So you can't fix malicious websites. What about websites that do this that are just dumb? Well, the mouseenter/mouseover thing is sort of weird anyway - what happens to keyboard or a11y tool users? They just go to the wrong link? You can kind of sort of try to deal with that by adding an onfocus event listener as well, but you'd still screw over users without JS enabled or who use a11y tools, or enter reader mode before trying to navigate the link, or...

So personally, I would suggest wontfix. If the website intends to be obnoxious about what URL will load when the user clicks the link, there isn't a reasonable way for us to stop it doing that. If the website intends for the URL to go somewhere else without being obnoxious about it, it should set it in a less fragile way. If we want to care more about that second case, we could invest the time and add the complexity to dom/docshell code to do that. It's up to the DOM team. :-)

Comment 5

[Victor Engmark]

I think it's worth noting that I first noticed this on Google, where every search result URL is changed onmousedown to a tracking URL. That use case alone makes this worth some work and even slowness.

Comment 6

[:Gijs (he/him)]

(In reply to Victor Engmark from comment #5)
> I think it's worth noting that I first noticed this on Google, where every
> search result URL is changed onmousedown to a tracking URL.

That's not the same as the attached testcase, and depending on the approach wouldn't necessarily be fixed by changes that fix this testcase...

It also wouldn't be noticeable to the user if they were clicking the URL, unless they wait a long time before mouseup to inspect the link (which in practice seems unlikely - and as I already noted in comment #4, wouldn't help if the site was intent on hiding the fact that it changes the URL.

> That use case
> alone makes this worth some work and even slowness.

While you're obviously free to hold this opinion, I don't see how it follows from the available evidence. It's not clear to me how even if we fixed the google mousedown case, that would help users in any meaningful way. What practical problem would it solve? It can't reliably solve "what is the target of this link", it mostly won't be noticed by users as there's so little time between mousedown and navigation happening, and there's nothing the user can actually *do* about the link changing, either (not without writing custom JS, anyway).

If Google changing these URLs is sufficiently annoying, if anything that would be an argument for enabling <a ping> support by default, which is what they rely on for tracking in Chrome and presumably Firefox having it preffed off by default is why they serve it different markup + JS.

Comment 7

[Marcos Caceres [:marcosc]]

Thanks :Gijs for the additional info. Ehsan, Gijs makes some pretty compelling points in https://bugzilla.mozilla.org/show_bug.cgi?id=1438660#c4 towards won't fix.   

However, as we've been doing triage over the last few weeks, we've seen issues related to the status bar not reflecting `href` changes come up. So, we could decide to just won't fix any of them or consider investing a bit of time (I agree with Gijs that the click case from Google seems like a lost cause).

Comment 8

[Andrew Overholt [:overholt]]

I have nothing useful to add to the discussion here :)

Comment 9

[(no longer active)]

I agree with Gijs that fixing this will do nothing to address the problem for Google search, mostly due to the short window of time between pressing down the mouse and releasing it for most users.

But I disagree that we *shouldn't* fix this, or that fixing this is particularly tricky.  In fact fixing this could be done quite simply by extending nsDocShell::OnOverLink() to optionally register a listener for changes to the href of the link passed to it and for nsDocShell::OnLeaveLink() to clear it.  I don't think we need to keep track of the last element passed to OnOverLink() or anything that sophisticated.

In summary, I think this is a small bug worth fixing at some point, but not all that important to fix unless we have evidence that it affects a major website.

Comment 10

[Marcos Caceres [:marcosc]]

Thanks :Ehsan! Ok, let's keep this as P3.

Comment 13

[:Gijs (he/him)]

Copying over this comment from bug 1433319 in case it's useful:

(In reply to Paul from bug 1433319 comment #8)
> The reporter of bug #1468727 has created an onmouseover test
> https://www.w3schools.com/code/tryit.asp?filename=FSB2DJHIGFTV which
> demonstrates the following variations on this behaviour in Firefox 60:
> 
> 1. Does not update the preview link on hover
> 2. Does change the preview link on click, but only the first time
> 3. Doesn't change the preview link on subsequent clicks until the page is
> reloaded
> 4. Always shows the correct preview link post facto if tapped using a
> touchscreen