window.onbeforeunload is called when clicking download links |
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.51 Safari/537.36 Steps to reproduce the problem: 1. Add a callback to window.onbeforeunload 2. Have an download link <a download="file.txt"> 3. Click the download link What is the expected behavior? onbeforeunload is not called for download links. What went wrong? onbeforeunload is called for download links. Did this work before? Yes 64.0.3282.140 Does this work in other browsers? Yes Chrome version: 65.0.3325.51 Channel: beta OS Version: Flash Version:
,
Feb 15 2018
,
Feb 15 2018
blois@ Thanks for the issue. Tested this issue on Ubuntu 14.04, Mac OS 10.12.6 and Windows 10 on the Beta 65.0.3325.73 and Canary 66.0.3347.0 and able to reproduce the issue by following the steps mentioned above. Bisect Information: =================== Good Build: 65.0.3309.0 (Revision - 526409) Bad Build : 65.0.3310.0 (Revision - 526575) On executing the per-revision bisect script, below is the Changelog URL: https://chromium.googlesource.com/chromium/src/+log/cbce1149c9ae08297b816effbf456c02f97df8db..f2d2fe87028de36a489f7db3f5fb28da2e9d9b2b From the above Changelog, suspecting the below change: Reviewed-on: https://chromium-review.googlesource.com/758236 jochen@ Please check and confirm if this issue is related to your change, else help us in assigning to the right owner. CC'ing the reviewer of this issue clamy@, as this issue is a RBS issue and the author is OOO till the 18th of this month Adding ReleaseBlock-Stable as this is a recent regression. Please feel to remove the same if it is not applicable. Thanks.
,
Feb 18 2018
note that for general URLS, this is expected (also repros in Firefox). It seems that for blob URLs, there's a special case where it doesn't fire this event.
,
Feb 20 2018
After talking to Anne, I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1439498 - let's wait what they think
,
Feb 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd05cee6edd1301f63d44306ff120820b6a4cfde commit cd05cee6edd1301f63d44306ff120820b6a4cfde Author: Jochen Eisinger <jochen@chromium.org> Date: Mon Feb 26 10:04:14 2018 Don't dispatch beforeunload events for downloads BUG= 812293 Change-Id: I1375ecbdef13ea43ac350cdae6e550267166fadc Reviewed-on: https://chromium-review.googlesource.com/934204 Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Jochen Eisinger <jochen@chromium.org> Cr-Commit-Position: refs/heads/master@{#539097} [modify] https://crrev.com/cd05cee6edd1301f63d44306ff120820b6a4cfde/content/renderer/render_frame_impl.cc [delete] https://crrev.com/7b761842aab4a59c63b6e125e56c88eb1de55932/third_party/WebKit/LayoutTests/external/wpt/html/semantics/embedded-content/the-area-element/area-download-click-expected.txt [modify] https://crrev.com/cd05cee6edd1301f63d44306ff120820b6a4cfde/third_party/WebKit/LayoutTests/external/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt [delete] https://crrev.com/7b761842aab4a59c63b6e125e56c88eb1de55932/third_party/WebKit/LayoutTests/external/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-expected.txt
,
Feb 26 2018
,
Feb 27 2018
Will you merge it to Chrome 65?
,
Feb 28 2018
Issue 817009 has been merged into this issue.
,
Feb 28 2018
jochen@ Merged the issue 817009 to this issue as it is similar. Could you please merge the fix to M-65, as Stable is soon moving to M-65. Hence adding ReleaseBlock-Stable. Please feel free to remove if this is not applicable. Thanks..
,
Feb 28 2018
//Adding to comment #11. Able to reproduce this issue on Windows 10, Mac OS 10.12.6 and Ubuntu 14.04 on Beta 65.0.3325.88 and Dev 66.0.3355.0. Issue seems to be fixed on Canary 66.0.3356.0 and Stable 64.0.3282.186. Thanks..
,
Feb 28 2018
,
Feb 28 2018
This bug requires manual review: We are only 5 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 28 2018
Before we approve merge to M65, could you pls confirm followings? Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge? Any other imp details to justify the merge. Please note M65 is VERY close to Stable promotion so merge bar is very high. Thank you.
,
Feb 28 2018
It's on today's canary. I think it's safe to merge, and has tests, however, given that the initial bug was only reported well into the beta period, I'm not sure how to quantify "baked". I'm confident that the fix won't introduce crashers, nor will it make stuff worse than the reported bug.
,
Feb 28 2018
,
Feb 28 2018
Approving merge to M65 branch based on comments #12, #16 and per offline chat with jochen@. +cmasso@ (Chrome on Android) TPM to keep her in loop.
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5f8ea0c6058415c9d64a5a1e36f3d95b610ae2c8 commit 5f8ea0c6058415c9d64a5a1e36f3d95b610ae2c8 Author: Jochen Eisinger <jochen@chromium.org> Date: Wed Feb 28 16:23:46 2018 Don't dispatch beforeunload events for downloads BUG= 812293 TBR=jochen@chromium.org (cherry picked from commit cd05cee6edd1301f63d44306ff120820b6a4cfde) Change-Id: I1375ecbdef13ea43ac350cdae6e550267166fadc Reviewed-on: https://chromium-review.googlesource.com/934204 Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Jochen Eisinger <jochen@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#539097} Reviewed-on: https://chromium-review.googlesource.com/941446 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#623} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/5f8ea0c6058415c9d64a5a1e36f3d95b610ae2c8/content/renderer/render_frame_impl.cc [delete] https://crrev.com/a943a3711b1cbba1b07b0f16c279b2fddf515acb/third_party/WebKit/LayoutTests/external/wpt/html/semantics/embedded-content/the-area-element/area-download-click-expected.txt [modify] https://crrev.com/5f8ea0c6058415c9d64a5a1e36f3d95b610ae2c8/third_party/WebKit/LayoutTests/external/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt [delete] https://crrev.com/a943a3711b1cbba1b07b0f16c279b2fddf515acb/third_party/WebKit/LayoutTests/external/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-expected.txt |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by blois@google.com
, Feb 14 2018