New issue
Advanced search Search tips

Issue 845824 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 11
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug



Sign in to add a comment

Align with the HTML Standard on fragment navigation

Project Member Reported by annevank...@gmail.com, May 23 2018

Issue description

See new tests over at https://github.com/w3c/web-platform-tests/pull/8723. Corresponding HTML Standard change is at https://github.com/whatwg/html/pull/3111.
 

Comment 1 by tkent@chromium.org, May 25 2018

Labels: Needs-BlinkIntent
Status: Available (was: Unconfirmed)
Labels: -Needs-BlinkIntent OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Windows
I changed my mind.  We don't need an intent-to-remove for ScrollToFragmentSucceedWithMixed removal.  Other browsers don't have this behavior, it doesn't follow the specification, and its UseCounter is only 0.001326%.

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 26

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

commit 3cca7e0c6f085e99e284e7ead1360e675b901df0
Author: Kent Tamura <tkent@chromium.org>
Date: Thu Jul 26 00:55:11 2018

url: DecodeURLEscapeSequences() should not apply UTF-8 and isomorphic encoding to a single input.

DecodeURLEscapeSequences() decoded some parts of an input string in UTF-8,
and other parts of the string in isomorphic encoding. This behavior doesn't
make sense, is harmful against browser interoperability, and is used only
in 0.0013% of page views (only in scroll-to-fragment cases).

DecodeURLEscapeSequences() is used in many code paths, and we have usage
data only for scroll-to-framgnet. However this CL tries to remove this
weird behavior entirely.


Bug:  845824 
Change-Id: Ib1cbbd3e1c26ebaaca7b3537b9732121835fdc9f
Reviewed-on: https://chromium-review.googlesource.com/1146535
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578151}
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/third_party/WebKit/LayoutTests/external/wpt/html/browsers/browsing-the-web/scroll-to-fragid/fragment-and-encoding-expected.txt
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/third_party/WebKit/LayoutTests/external/wpt/html/browsers/browsing-the-web/scroll-to-fragid/fragment-and-encoding.html
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/third_party/blink/renderer/platform/weborigin/kurl_test.cc
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/url/url_util.cc
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/url/url_util.h
[modify] https://crrev.com/3cca7e0c6f085e99e284e7ead1360e675b901df0/url/url_util_unittest.cc

The remaining task is to remove ScrollToFragmentSucceedWithIsomorphic behavior.

This is also not in other major browsers, and its UseCounter is 0.003%.  We may assume the removal is just a bug fix.

Interoperability:

Curent Chrome:
1. As-is (no percent-decode)
2. utf8-decode(percent-decode(fragment)) or isomorphic-decode(percent-decode(fragment))
(Try isomorphic-decode only if utf8-decode fails)

Edge
1. As-is (no percent-decode)
2. utf8-decode(percent-decode(fragment))

Safari:
1. As-is (no percent-decode)
2. page-encoding-decode(percent-decode(fragment))

Firefox
1. As-is (no percent-decode)
2. utf8-decode(percent-decode(fragment))
3. page-encoding-decode(percent-decode(fragment))

HTML specification:
1. As-is (no percent-decode)
2. utf8-decode(percent-decode(fragment))

Cc: -tkent@chromium.org
Owner: tkent@chromium.org
Status: Started (was: Available)
https://chromium-review.googlesource.com/c/chromium/src/+/1333178
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 11

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

commit 703db889020eaed8a0cd525f8c5a6be76186983d
Author: Kent Tamura <tkent@chromium.org>
Date: Tue Dec 11 04:00:55 2018

Drop isomorphic decoding from fragment navigation

When Chrome opens a URL with a fragment id, it decodes %xx for UTF-8,
and decodes it for isomorphic-decode [1] if it is invalid for UTF-8.
Then tries to find an element with the decoding result as an ID.

No other browsers do isomorphic-decode, and it's not defined by the
standard.  This CL removes this non-standard behavior.

This CL doesn't apply Blink's intent process. This is a very small
behavior change, and affects only 0.0016% of page views.

* Implementation
 - Add a flag to disable isomorphic-decode to
url::DecodeURLEscapeSequences(), and apply the new behavior only to
blink::LocalFrameView::ProcessUrlFragment().

 - Remove unnecessary UseCounters.


chromestatus: https://www.chromestatus.com/feature/4885685374812160
[1] https://infra.spec.whatwg.org/#isomorphic-decode

Change-Id: I17788e3697484701db66dbfef53bd1cae4598c51
Bug:  845824 
Reviewed-on: https://chromium-review.googlesource.com/c/1333178
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: John Chen <johnchen@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: David Jacobo <djacobo@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615419}
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/android_webview/renderer/aw_render_frame_ext.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/chrome/common/media_router/providers/cast/cast_media_source.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/chrome/test/chromedriver/server/http_handler.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/components/arc/intent_helper/link_handler_model.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/content/common/content_security_policy/csp_source.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/bindings/core/v8/script_controller.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/css/resolver/element_style_resources.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/exported/web_plugin_container_impl.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/fileapi/file.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/frame/csp/csp_source.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/frame/csp/source_list_directive.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/frame/location.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/html/image_document.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/html/media/media_fragment_uri_parser.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/loader/form_submission.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/page/create_window.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/svg/svg_resource.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/svg/svg_uri_reference.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/svg/svg_use_element.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/core/url/url_search_params.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/platform/weborigin/kurl.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/platform/weborigin/kurl.h
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/platform/weborigin/kurl_test.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/third_party/blink/renderer/platform/weborigin/security_origin.cc
[delete] https://crrev.com/9639de0797593971c5b56e3cecefcac60fd86d28/third_party/blink/web_tests/external/wpt/html/browsers/browsing-the-web/scroll-to-fragid/fragment-and-encoding-2-expected.txt
[delete] https://crrev.com/9639de0797593971c5b56e3cecefcac60fd86d28/third_party/blink/web_tests/external/wpt/html/browsers/browsing-the-web/scroll-to-fragid/fragment-and-encoding-expected.txt
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/url/url_util.cc
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/url/url_util.h
[modify] https://crrev.com/703db889020eaed8a0cd525f8c5a6be76186983d/url/url_util_unittest.cc

Labels: Target-73
Status: Fixed (was: Started)

Sign in to add a comment