Closed Bug 1382035 Opened 7 years ago Closed 6 years ago

Getting a promise in a javascript: href link navigates away from the page unless the returned promise is handled.

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed

People

(Reporter: wisniewskit, Assigned: bzbarsky)

References

Details

(Keywords: regression, triage-deferred, Whiteboard: [webcompat])

Attachments

(2 files)

Attached file testcase.html
When using a link with this form:

    <a href="javascript:">

If the javascript: code contains a video.play() call that is not carefully handled in some way, Firefox will navigate the tab away to a page with just the plain text "[object Promise]".

For instance, the attached test-case is a reduced case of what was recently found on Best Buy's page, and contains this link to start playing a nested video tag:

    <a href="javascript: { var vid = document.getElementById('video_1943544786'); if (!vid.loaded) { vid.load(); vid.loaded = true; } if (vid.paused) vid.play(); else vid.pause(); }">

Chrome, Safari and Edge simply load and play the video on the first click/tap. Firefox navigates away to an "[object Promise]" page. One must deal with the play() call in some way (pass it to alert, await its promise, or just "return false" at the end of the script) for Firefox to behave as the other browsers do.

This strikes me as a bug/interop issue which should be fixed. I'm not sure if it's related to the JS engine or DOM, so I've posted it in the JS component to start off with.
Flags: webcompat?
Mozregression tells me that this "broke" when video.play() was made to return a promise (bug 1244768). In addition, when I have the code deal with *any* promise, even one made by calling a based JS function as below, then the page still navigates away. For instance the below case takes me to a page saying "false" instead of "[object Promise]":

  <!DOCTYPE html><html><body>
    <script>function getPromise() { return new Promise(() => {}); };</script>
    <a href="javascript: getPromise() && false">test</a>
  </body></html>

So this isn't specific to video.play(). One must explicitly "return false" or appear to handle the promise for the navigation to not occur, so I'm guessing this is related to how we deal with promises which appear to be unhandled.
Summary: Using video.play() in a javascript: href link navigates away from the page unless the returned promise is handled. → Getting a promise in a javascript: href link navigates away from the page unless the returned promise is handled.
Keywords: triage-deferred
Priority: -- → P3
Let's try DOM.
Component: JavaScript Engine → DOM
Flags: webcompat?
Priority: P3 → --
Boris, can you take a quick look here and see if we're doing something non-standard?
Flags: needinfo?(bzbarsky)
Priority: -- → P2
Well, the first problem here is that what the HTML spec says to do right now is known to not be web-compatible, as we discovered when we tried to implement it.  See https://github.com/whatwg/html/issues/1895 and https://github.com/whatwg/html/issues/1896

For extra fun, Chrome treats `<a href='javascript:'>` differently from `<iframe src='javascript:'>`, as mentioned in https://github.com/whatwg/html/issues/1896#issuecomment-253948371

So that's the main thing going on here: various sites are depending on the <iframe> behavior, we are more or less aligned with Chrome on the <iframe> behavior, we have the same behavior for both <iframe> and <a> (which sort of follows spec insofar as the spec can be followed and sanity), Chrome follows neither.

Domenic, Anne, can we please get these spec issues resolved?

Failing the spec starting to make sense here, what we could do is explicitly treat promises like undefined in javascript: return values.  There's really no good reason to stringify them there, and in cases like this (where the promise really does represent undefined) it makes perfect sense to do it.
Flags: needinfo?(d)
Flags: needinfo?(bzbarsky)
Flags: needinfo?(annevk)
Thomas, was this causing compat issues in the wild?
Flags: needinfo?(wisniewskit)
Yes, it is at least known to cause videos to not play on Best Buy's Geek Squad page, as per https://webcompat.com/issues/5572
Flags: needinfo?(wisniewskit)
OK.  Then we should do something now, and wait for the spec to catch up.

At the same time, I would rather not thrash our behavior too much, so would rather not change behaviors more than once for various return values.

Given that, I am going to explicitly special-case Promise here for now.  It's pretty clear that long-term any solution here will involve Promise return values not performing a navigation.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8993572 [details] [diff] [review]
Treat Promise return values like undefined for javascript: urls

Review of attachment 8993572 [details] [diff] [review]:
-----------------------------------------------------------------

Can you get a bug on file (in our bugzilla) about resolving the spec issues and doing any followup work, and refer to it in a comment in the code? I guess the followup might be just to remove the code comment again, depending on how the spec issues are resolved.
Attachment #8993572 - Flags: review?(peterv) → review+
> Can you get a bug on file (in our bugzilla) about resolving the spec issues and doing any followup work

Done, bug 1477821.
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f78090c350c
Treat Promise return values like undefined for javascript: urls.  r=peterv
https://hg.mozilla.org/mozilla-central/rev/3f78090c350c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Further spec discussion at https://github.com/whatwg/html/issues/1896#issuecomment-409726397 ; clearing my needinfo
Flags: needinfo?(d)
I assume we want this to ride the trains, but feel free to nominate for Beta approval if you feel strongly otherwise.
Given that this "broke" in Firefox 53, I'm not really sure there a strong argument for backporting the fix...  I guess mostly depends on whether we're OK with the Geek Squad page being broken for an extra release.
Seems reasonable to let this ride the trains.
Flags: needinfo?(annevk)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: