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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla63
People
(Reporter: wisniewskit, Assigned: bzbarsky)
References
Details
(Keywords: regression, triage-deferred, Whiteboard: [webcompat])
Attachments
(2 files)
531 bytes,
text/html
|
Details | |
4.55 KB,
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
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?
Reporter | ||
Comment 1•7 years ago
|
||
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.
Updated•7 years ago
|
Keywords: triage-deferred
Priority: -- → P3
Comment 2•6 years ago
|
||
Let's try DOM.
Component: JavaScript Engine → DOM
Flags: webcompat?
Priority: P3 → --
Updated•6 years ago
|
Keywords: regression
Comment 3•6 years ago
|
||
Boris, can you take a quick look here and see if we're doing something non-standard?
Flags: needinfo?(bzbarsky)
Priority: -- → P2
Assignee | ||
Comment 4•6 years ago
|
||
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)
Assignee | ||
Comment 5•6 years ago
|
||
Thomas, was this causing compat issues in the wild?
Flags: needinfo?(wisniewskit)
Comment 6•6 years ago
|
||
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)
Assignee | ||
Comment 7•6 years ago
|
||
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 | ||
Comment 8•6 years ago
|
||
Attachment #8993572 -
Flags: review?(peterv)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment 9•6 years ago
|
||
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+
Assignee | ||
Comment 10•6 years ago
|
||
> Can you get a bug on file (in our bugzilla) about resolving the spec issues and doing any followup work Done, bug 1477821.
Comment 11•6 years ago
|
||
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
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3f78090c350c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment 13•6 years ago
|
||
Further spec discussion at https://github.com/whatwg/html/issues/1896#issuecomment-409726397 ; clearing my needinfo
Flags: needinfo?(d)
Comment 14•6 years ago
|
||
I assume we want this to ride the trains, but feel free to nominate for Beta approval if you feel strongly otherwise.
status-firefox61:
--- → wontfix
status-firefox62:
--- → wontfix
status-firefox-esr52:
--- → wontfix
status-firefox-esr60:
--- → wontfix
Assignee | ||
Comment 15•6 years ago
|
||
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.
Comment 16•6 years ago
|
||
Seems reasonable to let this ride the trains.
Updated•6 years ago
|
Flags: needinfo?(annevk)
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•