Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2760013002:
DevTools: Fixes regression in cookies tab in Network panel having no output (Closed)
|
Created:
3 years, 9 months ago by jrstanley Modified:
3 years, 9 months ago CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, pfeldman Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevTools: Fixes regression in cookies tab in Network panel having no output
Clicking on a request in the Network panel and selecting the cookies tab resulted in no output in this tab, despite there being cookies set.
This appears to be a regression in https://codereview.chromium.org/2714913002
BUG=702940
Review-Url: https://codereview.chromium.org/2760013002
Cr-Commit-Position: refs/heads/master@{#458847}
Committed: https://chromium.googlesource.com/chromium/src/+/b8fba9d8708e06e1543bc3d9fe589640c8853137
Patch Set 1 #
Total comments: 1
Patch Set 2 : Updates JSDoc comment to reflect possible null cookies parameter #
Total comments: 1
Patch Set 3 : Rebase (I'm now already in the AUTHORS file) #Messages
Total messages: 34 (22 generated)
james@apphaus.co.uk changed reviewers: + dgozman@chromium.org, eostroukhov@chromium.org
PTAL. Apologies in advance if I have selected inappropriate reviewers.
https://codereview.chromium.org/2760013002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/cookie_table/CookiesTable.js (right): https://codereview.chromium.org/2760013002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/cookie_table/CookiesTable.js:168: if (!cookies) Please fix JSDoc comment above to indicate that cookies might be null (replace first exclamation point with the question mark)
On 2017/03/20 at 17:34:59, eostroukhov wrote: > https://codereview.chromium.org/2760013002/diff/1/third_party/WebKit/Source/d... > File third_party/WebKit/Source/devtools/front_end/cookie_table/CookiesTable.js (right): > > https://codereview.chromium.org/2760013002/diff/1/third_party/WebKit/Source/d... > third_party/WebKit/Source/devtools/front_end/cookie_table/CookiesTable.js:168: if (!cookies) > Please fix JSDoc comment above to indicate that cookies might be null (replace first exclamation point with the question mark) Done - thanks eostroukhov. PTAL.
https://codereview.chromium.org/2760013002/diff/20001/AUTHORS File AUTHORS (right): https://codereview.chromium.org/2760013002/diff/20001/AUTHORS#newcode308 AUTHORS:308: James Stanley <james@apphaus.co.uk> Have you signed CLA? https://cla.developers.google.com/clas Thanks!
On 2017/03/20 at 17:57:47, eostroukhov wrote: > https://codereview.chromium.org/2760013002/diff/20001/AUTHORS > File AUTHORS (right): > > https://codereview.chromium.org/2760013002/diff/20001/AUTHORS#newcode308 > AUTHORS:308: James Stanley <james@apphaus.co.uk> > Have you signed CLA? https://cla.developers.google.com/clas > > Thanks! No problem, thank you. Yes the CLA has been signed.
dgozman@chromium.org changed reviewers: + phulce@chromium.org
Patrick, could you please double check for more regressions caused by that patch? We'll have to merge the fix to M58, so let's be sure nothing else crept in.
Thanks for the fix, and sorry about this! I'll add some testing to the network cookies panel and a few more to general cookies functionality as well.
lgtm
The CQ bit was checked by james@apphaus.co.uk to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by james@apphaus.co.uk to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder probably lacks capacity)
The CQ bit was checked by james@apphaus.co.uk to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder probably lacks capacity)
The CQ bit was checked by james@apphaus.co.uk to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by phulce@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org Link to the patchset: https://codereview.chromium.org/2760013002/#ps40001 (title: "Rebase (I'm now already in the AUTHORS file)")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/03/22 18:54:15, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... Patrick, let's make sure we merge this to 58.
On 2017/03/22 18:54:51, dgozman wrote: > On 2017/03/22 18:54:15, commit-bot: I haz the power wrote: > > CQ is trying da patch. Follow status at > > > > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... > > Patrick, let's make sure we merge this to 58. Yep, I'll own it.
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490208822576200,
"parent_rev": "ea03b65c2f654b87e4033af550c4c295fb76747d", "commit_rev":
"b8fba9d8708e06e1543bc3d9fe589640c8853137"}
Message was sent while issue was closed.
Description was changed from ========== DevTools: Fixes regression in cookies tab in Network panel having no output Clicking on a request in the Network panel and selecting the cookies tab resulted in no output in this tab, despite there being cookies set. This appears to be a regression in https://codereview.chromium.org/2714913002 BUG=702940 ========== to ========== DevTools: Fixes regression in cookies tab in Network panel having no output Clicking on a request in the Network panel and selecting the cookies tab resulted in no output in this tab, despite there being cookies set. This appears to be a regression in https://codereview.chromium.org/2714913002 BUG=702940 Review-Url: https://codereview.chromium.org/2760013002 Cr-Commit-Position: refs/heads/master@{#458847} Committed: https://chromium.googlesource.com/chromium/src/+/b8fba9d8708e06e1543bc3d9fe58... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/b8fba9d8708e06e1543bc3d9fe58... |
