externalnav handler doesnt anchor youtube regex |
|||
Issue descriptionIs it expected that this regex isn't anchored? (e.g. it will match "youtube.com.foo.com?pairingCode=xyz") https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationHandler.java?type=cs&l=331 // Special case - It makes no sense to use an external application for a YouTube // pairing code URL, since these match the current tab with a device (Chromecast // or similar) it is supposed to be controlling. Using a different application // that isn't expecting this (in particular YouTube) doesn't work. if (params.getUrl().matches(".*youtube\\.com.*[?&]pairingCode=.*")) { if (DEBUG) Log.i(TAG, "NO_OVERRIDE: YouTube URL with a pairing code"); return OverrideUrlLoadingResult.NO_OVERRIDE; } I don't understand the context, so maybe this is WAI or not a bug.
,
Nov 2 2017
I suspect it's an oversight, but it's harmless since it just keeps the navigation in Chrome, which is the default behaviour anyway.
,
Feb 2 2018
,
Mar 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cb52b532908eb01d0dec72cdc0934f581555e9e commit 5cb52b532908eb01d0dec72cdc0934f581555e9e Author: Troy Hildebrandt <thildebr@chromium.org> Date: Tue Mar 06 16:36:46 2018 Tighten up regex for youtube pairingCode URLs in ExternalNavHandler. Links that worked prior to fix: OK: http://m.youtube.com/watch?v=1234&pairingCode=5678 youtube.com?pairingCode=xyz youtube.com/tv?pairingCode=xyz youtube.com/watch?v=1234&version=3&autohide=1&pairingCode=xyz youtube.com/watch?v=1234&pairingCode=xyz&version=3&autohide=1 BAD: youtube.com.foo.com/tv?pairingCode=xyz youtube.com.foo.com?pairingCode=xyz youtube.com/watch?v=tEsT&version=3&autohide=1&pairingCode= youtube.com&pairingCode=xyz youtube.com/watch?v=tEsT?version=3&pairingCode=&autohide=1 This CL fixes the regex that filters youtube pairing code URLs, and now correctly filters those listed under BAD. The new regex allows for the pairingCode to be anywhere in the URL's parameters, but it can't be empty. Bug: 780979 Change-Id: Icaa279045bc654077cbdb4edcf70c586e476a1bc Reviewed-on: https://chromium-review.googlesource.com/941584 Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org> Commit-Queue: Troy Hildebrandt <thildebr@chromium.org> Cr-Commit-Position: refs/heads/master@{#541119} [modify] https://crrev.com/5cb52b532908eb01d0dec72cdc0934f581555e9e/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationHandler.java [modify] https://crrev.com/5cb52b532908eb01d0dec72cdc0934f581555e9e/chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationHandlerTest.java
,
Mar 7 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by wfh@chromium.org
, Nov 2 2017Labels: -Pri-3 Pri-2
Owner: mariakho...@chromium.org
Status: Assigned (was: Unconfirmed)