61868 2011-06-01 09:49:46 -0700 Web Inspector: ResourceCookiesView.resize() missing 2011-06-02 18:21:43 -0700 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED Regression P2 Normal --- 1 caseq caseq apavlov bweinstein commit-queue joepeck keishi loislo pfeldman pmuellr rik timothy yurys oldest_to_newest 413447 0 caseq 2011-06-01 09:49:46 -0700 1. Open inspector 2. Navigate to http://www.webkit.org 3. Switch to network panel 4. Select main resource 5. Switch to cookies view 6. Resize WebInspector window Observer that columns can no longer be resized, the following error appears on inspector's console: Uncaught TypeError: Object #<Object> has no method 'resize' 413449 1 95615 caseq 2011-06-01 09:52:10 -0700 Created attachment 95615 patch 413586 2 95615 pfeldman 2011-06-01 12:34:03 -0700 Comment on attachment 95615 patch View in context: https://bugs.webkit.org/attachment.cgi?id=95615&action=review > Source/WebCore/inspector/front-end/ResourceCookiesView.js:64 > + this._cookiesTable.updateWidths(); Can resize() be called before show()? I.e. are you missing the null check here? 413686 3 caseq 2011-06-01 14:49:54 -0700 (In reply to comment #2) > (From update of attachment 95615 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95615&action=review > > > Source/WebCore/inspector/front-end/ResourceCookiesView.js:64 > > + this._cookiesTable.updateWidths(); > > Can resize() be called before show()? I.e. are you missing the null check here? So far we only call it in case the view is already visible (http://www.google.com/codesearch?hl=en&vert=chromium&lr=&q=cookiesView.resize&sbtn=Search). Whether we should do a guard check for future usages is a matter of taste -- I'd rather not. We shouldn't be resizing something that we're not showing at the moment. 413914 4 95615 pfeldman 2011-06-01 22:22:39 -0700 Comment on attachment 95615 patch View in context: https://bugs.webkit.org/attachment.cgi?id=95615&action=review >>> Source/WebCore/inspector/front-end/ResourceCookiesView.js:64 >>> + this._cookiesTable.updateWidths(); >> >> Can resize() be called before show()? I.e. are you missing the null check here? > > So far we only call it in case the view is already visible (http://www.google.com/codesearch?hl=en&vert=chromium&lr=&q=cookiesView.resize&sbtn=Search). Whether we should do a guard check for future usages is a matter of taste -- I'd rather not. We shouldn't be resizing something that we're not showing at the moment. ResourceCookiesView inherits from View, so it should probably behave as a view. Views can get resize signals at any time while visible. I don't see synchronous correlation between showing view and getting this._cookiesTable assigned. In fact, if our tabbed pane was properly implemented and did delegate resize to the tabs, your code would fail on the cookie-less resource resize, right? 414029 5 95750 caseq 2011-06-02 04:09:26 -0700 Created attachment 95750 patch 414279 6 95750 pfeldman 2011-06-02 11:10:42 -0700 Comment on attachment 95750 patch We probably still want to fix tabbed pane so that it was a view and was calling resize of the selected view... 414588 7 95750 commit-queue 2011-06-02 18:21:38 -0700 Comment on attachment 95750 patch Clearing flags on attachment: 95750 Committed r87981: <http://trac.webkit.org/changeset/87981> 414589 8 commit-queue 2011-06-02 18:21:43 -0700 All reviewed patches have been landed. Closing bug. 95615 2011-06-01 09:52:10 -0700 2011-06-02 04:09:26 -0700 patch cookies-resize.diff text/plain 1200 caseq ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAzNzExNjFkLi5hMmJlN2NhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMg QEAKKzIwMTEtMDYtMDEgIEFuZHJleSBLb3N5YWtvdiAgPGNhc2VxQGNocm9taXVtLm9yZz4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXZWIgSW5zcGVj dG9yOiBSZXNvdXJjZUNvb2tpZXNWaWV3LnJlc2l6ZSgpIGlzIG1pc3NpbmcKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYxODY4CisKKyAgICAgICAgKiBp bnNwZWN0b3IvZnJvbnQtZW5kL1Jlc291cmNlQ29va2llc1ZpZXcuanM6CisgICAgICAgIChXZWJJ bnNwZWN0b3IuUmVzb3VyY2VDb29raWVzVmlldy5wcm90b3R5cGUucmVzaXplKToKKwogMjAxMS0w Ni0wMSAgSmF5IENpdmVsbGkgIDxqY2l2ZWxsaUBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2 aWV3ZWQgYnkgQWRhbSBCYXJ0aC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rv ci9mcm9udC1lbmQvUmVzb3VyY2VDb29raWVzVmlldy5qcyBiL1NvdXJjZS9XZWJDb3JlL2luc3Bl Y3Rvci9mcm9udC1lbmQvUmVzb3VyY2VDb29raWVzVmlldy5qcwppbmRleCAwMWNiODgyLi41NWM1 MmIxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL1Jlc291 cmNlQ29va2llc1ZpZXcuanMKKysrIGIvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVu ZC9SZXNvdXJjZUNvb2tpZXNWaWV3LmpzCkBAIC01OSw2ICs1OSwxMSBAQCBXZWJJbnNwZWN0b3Iu UmVzb3VyY2VDb29raWVzVmlldy5wcm90b3R5cGUgPSB7CiAgICAgICAgIHRoaXMuX2Nvb2tpZXNU YWJsZS51cGRhdGVXaWR0aHMoKTsKICAgICB9LAogCisgICAgcmVzaXplOiBmdW5jdGlvbigpCisg ICAgeworICAgICAgICB0aGlzLl9jb29raWVzVGFibGUudXBkYXRlV2lkdGhzKCk7CisgICAgfSwK KwogICAgIGdldCBfZ290Q29va2llcygpCiAgICAgewogICAgICAgICByZXR1cm4gISEodGhpcy5f cmVzb3VyY2UucmVxdWVzdENvb2tpZXMgfHwgdGhpcy5fcmVzb3VyY2UucmVzcG9uc2VDb29raWVz KTsK 95750 2011-06-02 04:09:26 -0700 2011-06-02 18:21:38 -0700 patch cookies-resize.diff text/plain 1237 caseq ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBhZThhYWUxLi5hYTlkOGUzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMg QEAKKzIwMTEtMDYtMDEgIEFuZHJleSBLb3N5YWtvdiAgPGNhc2VxQGNocm9taXVtLm9yZz4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXZWIgSW5zcGVj dG9yOiBSZXNvdXJjZUNvb2tpZXNWaWV3LnJlc2l6ZSgpIGlzIG1pc3NpbmcKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYxODY4CisKKyAgICAgICAgKiBp bnNwZWN0b3IvZnJvbnQtZW5kL1Jlc291cmNlQ29va2llc1ZpZXcuanM6CisgICAgICAgIChXZWJJ bnNwZWN0b3IuUmVzb3VyY2VDb29raWVzVmlldy5wcm90b3R5cGUucmVzaXplKToKKwogMjAxMS0w Ni0wMSAgRGFuIEJlcm5zdGVpbiAgPG1pdHpAYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2Vk IGJ5IEFuZGVycyBDYXJsc3Nvbi4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rv ci9mcm9udC1lbmQvUmVzb3VyY2VDb29raWVzVmlldy5qcyBiL1NvdXJjZS9XZWJDb3JlL2luc3Bl Y3Rvci9mcm9udC1lbmQvUmVzb3VyY2VDb29raWVzVmlldy5qcwppbmRleCAwMWNiODgyLi5jMTI2 YTkzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL1Jlc291 cmNlQ29va2llc1ZpZXcuanMKKysrIGIvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVu ZC9SZXNvdXJjZUNvb2tpZXNWaWV3LmpzCkBAIC01OSw2ICs1OSwxMiBAQCBXZWJJbnNwZWN0b3Iu UmVzb3VyY2VDb29raWVzVmlldy5wcm90b3R5cGUgPSB7CiAgICAgICAgIHRoaXMuX2Nvb2tpZXNU YWJsZS51cGRhdGVXaWR0aHMoKTsKICAgICB9LAogCisgICAgcmVzaXplOiBmdW5jdGlvbigpCisg ICAgeworICAgICAgICBpZiAodGhpcy5fY29va2llc1RhYmxlKQorICAgICAgICAgICAgdGhpcy5f Y29va2llc1RhYmxlLnVwZGF0ZVdpZHRocygpOworICAgIH0sCisKICAgICBnZXQgX2dvdENvb2tp ZXMoKQogICAgIHsKICAgICAgICAgcmV0dXJuICEhKHRoaXMuX3Jlc291cmNlLnJlcXVlc3RDb29r aWVzIHx8IHRoaXMuX3Jlc291cmNlLnJlc3BvbnNlQ29va2llcyk7Cg==