New issue
Advanced search Search tips

Issue 808966 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Unnecessary omnibox URL gets changed after capturing screenshot in Dev tools.

Reported by db...@etouch.net, Feb 5 2018

Issue description

Chrome Version : 66.0.3340.0 Revision cf4b77e4ccc0da994c6322f71863b0d17bb5c99f-refs/heads/master@{#534315}(32/64 bit)
OS: Win(7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.13.3) and Linux(14.04 LTS).
 
What steps will reproduce the problem?
(1) Launch chrome, open NTP and then open dev tools window on it.
(2) Click on 'Toggle device toolbar' and click on 'More options' and select 'Capture screenshot' option.
(3) Observe omnibox URL.

Actual: Unnecessary omnibox URL gets changed after capturing screenshot.

Expected: omnibox URL should not changed after capturing screenshot.

This is a regression issue, broken in 'M65', Using the per-revision bisect providing the bisect results,
Good Build:65.0.3309.0(Revision: 526410)
Bad Build: 65.0.3310.0(Revision: 526575)

You are probably looking for a change made after 526474 (known good), but no later than 526475 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  
https://chromium.googlesource.com/chromium/src/+log/cbce1149c9ae08297b816effbf456c02f97df8db..f2d2fe87028de36a489f7db3f5fb28da2e9d9b2b

Suspect: https://chromium.googlesource.com/chromium/src/+/f2d2fe87028de36a489f7db3f5fb28da2e9d9b2b

@Jochen Eisinger: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thank You!



 
Expected_URL.mp4
451 KB View Download

Comment 1 by db...@etouch.net, Feb 5 2018

Labels: RegressedIn-65 FoundIn-66 Target-66 Target-65 FoundIn-65
Actual_URL.mp4
424 KB View Download
Labels: ReleaseBlock-Stable
Adding release blocker label for this issue.Please reduce priority or remove if not the case.

Thank You!
Cc: creis@chromium.org
Charlie, can I pick your brain on this one?

It looks like if we navigate from the NTP and cancel the navigation, the cancelled URL will stay in the omnibox, even though we're still on the NTP.

Any idea why that is, or where I should start searching?

Comment 4 by creis@chromium.org, Feb 5 2018

Cc: pkasting@chromium.org
Components: UI>Browser>Navigation UI>Browser>Omnibox
Ah, yes-- that sounds related to the change we made in https://bugs.chromium.org/p/chromium/issues/detail?id=355537#c17, where we leave canceled URLs in the omnibox on the NTP to let the user edit and try again.  I'm not sure how it ties into this bug yet, but the relevant code is in NavigatorImpl::DiscardPendingEntryIfNeeded and Browser::ShouldPreserveAbortedURLs.  Hope that helps!
I wonder whether we should have more conditions, e.g., hide cancelled URLs if they're a non-web-accessible scheme, or whether it's just working as intended?

I think I'd lean towards the former (if not all non-web-accessible schemes, at least hide chrome-devtools: URLs)

Comment 6 by gov...@chromium.org, Feb 13 2018

M65 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge  into the release branch ASAP. Thank you.

Comment 7 by jochen@chromium.org, Feb 18 2018

Labels: -ReleaseBlock-Stable
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e

commit 9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e
Author: Jochen Eisinger <jochen@chromium.org>
Date: Wed Mar 07 16:44:01 2018

Hide navigations aborted due to downloading from ntp

If a navigation was started by an HTMLAnchorElement with a download
attribute (<a download>), the navigation will end up being aborted,
and turned into a download, so leaving the URL around for the user to
edit is not useful.

BUG= 808966 
R=creis@chromium.org,pkasting@chromium.org

Change-Id: Idcfa5c952cba7933fdf62e75a67ec4a2e2087695
Reviewed-on: https://chromium-review.googlesource.com/908451
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541471}
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/chrome/browser/download/download_ui_controller.cc
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/chrome/browser/ui/search/local_ntp_browsertest.cc
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/content/browser/frame_host/navigator.h
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/content/browser/frame_host/navigator_impl.h
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/9f9789e92c7adb2d86b91ec3f7c1bcce3770b77e/content/shell/browser/shell.cc

Status: Fixed (was: Assigned)

Sign in to add a comment