Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6)

Issue 2783623003: Fetch API: Don't exclude the URL fragment in Request#url (Closed)

Created:
3 years, 8 months ago by yiyix
Modified:
3 years, 7 months ago
Reviewers:
falken, sky, horo
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, kenjibaheux+watch_chromium.org, shimazu+serviceworker_chromium.org, serviceworker-reviews, blink-reviews-w3ctests_chromium.org, nhiroki, haraken, kinuko+serviceworker, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, tzik, varkha
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Fetch API: Update fetch request According to the spec, Request#url should not exclude the URL fragment (only Response#url excludes it). BUG=624278 Review-Url: https://codereview.chromium.org/2783623003 Cr-Commit-Position: refs/heads/master@{#461323} Committed: https://chromium.googlesource.com/chromium/src/+/7ea0952f2888c2de0cc0827743e63de9e7dc3778

Patch Set 1 #

Total comments: 6

Patch Set 2 : address comments #

Messages

Total messages: 43 (31 generated)
yiyix
Could you please take a look at this patch? Thank you.
3 years, 8 months ago (2017-03-31 05:17:38 UTC) #20
falken
lgtm CL description suggestions: "Fetch API: Update fetch request" -> "Fetch API: Don't exclude the ...
3 years, 8 months ago (2017-03-31 06:40:50 UTC) #22
yiyix
Thank for reviewing @falken @horo, could you please review changes in third_party/WebKit/Source/modules/fetch/Request.cpp? @sky, could you ...
3 years, 8 months ago (2017-03-31 07:24:03 UTC) #28
horo
lgtm Please add the links to the spec change commit and the spec discussion to ...
3 years, 8 months ago (2017-03-31 08:28:01 UTC) #29
falken
On 2017/03/31 08:28:01, horo wrote: > lgtm > > Please add the links to the ...
3 years, 8 months ago (2017-03-31 12:15:58 UTC) #30
sky
LGTM
3 years, 8 months ago (2017-03-31 14:29:07 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2783623003/80001
3 years, 8 months ago (2017-03-31 16:29:33 UTC) #34
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/340433)
3 years, 8 months ago (2017-03-31 18:44:19 UTC) #36
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2783623003/80001
3 years, 8 months ago (2017-04-01 12:56:55 UTC) #38
commit-bot: I haz the power
Committed patchset #2 (id:80001) as https://chromium.googlesource.com/chromium/src/+/7ea0952f2888c2de0cc0827743e63de9e7dc3778
3 years, 8 months ago (2017-04-01 13:52:04 UTC) #41
jeffcarp
On 2017/04/01 at 13:52:04, commit-bot wrote: > Committed patchset #2 (id:80001) as https://chromium.googlesource.com/chromium/src/+/7ea0952f2888c2de0cc0827743e63de9e7dc3778 GitHub PR ...
3 years, 8 months ago (2017-04-03 17:59:09 UTC) #42
falken
3 years, 7 months ago (2017-05-01 00:13:28 UTC) #43
Message was sent while issue was closed.
Created chrome status for this:
https://www.chromestatus.com/feature/4753419730419712

Powered by Google App Engine
This is Rietveld 408576698