50109 2010-11-26 06:20:15 -0800 Web Inspector: crash upon inspecting user style sheet. 2010-11-30 08:07:23 -0800 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 pfeldman pfeldman apavlov bweinstein commit-queue joepeck keishi loislo pfeldman pmuellr rik timothy yurys oldest_to_newest 313993 0 pfeldman 2010-11-26 06:20:15 -0800 Patch to follow. 313994 1 74929 pfeldman 2010-11-26 06:21:37 -0800 Created attachment 74929 [PATCH] Proposed change. 314172 2 74929 bweinstein 2010-11-28 02:46:41 -0800 Comment on attachment 74929 [PATCH] Proposed change. View in context: https://bugs.webkit.org/attachment.cgi?id=74929&action=review > WebCore/ChangeLog:7 > + Is there a reason resourceStyleSheetText crashes for user style sheets? A little more information here would be nice. > WebCore/inspector/InspectorStyleSheet.cpp:990 > + if (m_origin == "user" || m_origin == "user-agent") > + return false; It looks like canBind() in InspectorStyleSheet.h checks for "userAgent". Is there a difference between "user-agent" and "userAgent"? Should these checks match up? It would probably be nice to have those strings in constants as well. 314309 3 apavlov 2010-11-29 01:47:52 -0800 (In reply to comment #2) > (From update of attachment 74929 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=74929&action=review > > WebCore/inspector/InspectorStyleSheet.cpp:990 > > + if (m_origin == "user" || m_origin == "user-agent") > > + return false; > > It looks like canBind() in InspectorStyleSheet.h checks for "userAgent". Is there a difference between "user-agent" and "userAgent"? Should these checks match up? It would probably be nice to have those strings in constants as well. Yes, these two should match up. Pavel, can you please fix the occurrence to be "user-agent" (as required by CSSStyleModel.js)? Global constants would be really nice but it is quite tedious to set them up (as class static fields are not allowed in WebKit). Would be great if you could introduce them, though. 314348 4 apavlov 2010-11-29 04:14:30 -0800 (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 74929 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=74929&action=review > > > WebCore/inspector/InspectorStyleSheet.cpp:990 > > > + if (m_origin == "user" || m_origin == "user-agent") > > > + return false; > > > > It looks like canBind() in InspectorStyleSheet.h checks for "userAgent". Is there a difference between "user-agent" and "userAgent"? Should these checks match up? It would probably be nice to have those strings in constants as well. > > Yes, these two should match up. Pavel, can you please fix the occurrence to be "user-agent" (as required by CSSStyleModel.js)? Global constants would be really nice but it is quite tedious to set them up (as class static fields are not allowed in WebKit). Would be great if you could introduce them, though. On the second thought I figured that this would make user-agent styles editable, which will break the CSS editing experience (all editing operations will fail for these styles). To fix this, we should either support editing of styles not backed by source code or implement retrieval of user-agent stylesheet text, as we discussed last Friday offline (the latter is preferable, of course). 314506 5 74929 commit-queue 2010-11-29 10:37:08 -0800 Comment on attachment 74929 [PATCH] Proposed change. Clearing flags on attachment: 74929 Committed r72811: <http://trac.webkit.org/changeset/72811> 314507 6 commit-queue 2010-11-29 10:37:13 -0800 All reviewed patches have been landed. Closing bug. 314850 7 bweinstein 2010-11-29 21:16:05 -0800 I'm confused why this was marked r+ and cq+ when there were clear objections to the patch raised in the bug. 314851 8 bweinstein 2010-11-29 21:16:54 -0800 (In reply to comment #7) > I'm confused why this was marked r+ and cq+ when there were clear objections to the patch raised in the bug. Unless I missed some discussion offline that was relevant to the change being correct, I'm still not sure why there is an example of "userAgent" and "user-agent" in the .h and .cpp files. 315060 9 pfeldman 2010-11-30 08:07:23 -0800 (In reply to comment #8) > (In reply to comment #7) > > I'm confused why this was marked r+ and cq+ when there were clear objections to the patch raised in the bug. > > Unless I missed some discussion offline that was relevant to the change being correct, I'm still not sure why there is an example of "userAgent" and "user-agent" in the .h and .cpp files. Hey, sorry about that. This change was fixing a nasty crasher, so I decided to give it a go (it is #1 crasher in the chromium nightly, i suppose same would be reported from webkit nightlies). The idea was that the part that you noticed was immediately followed up on by Alexander. This did not happen though due to the misunderstanding. We will follow up immediately. Sorry again. 74929 2010-11-26 06:21:37 -0800 2010-11-29 10:37:07 -0800 [PATCH] Proposed change. style_crash text/plain 1114 pfeldman ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ZDFmYmM3OS4uNTIyZjc4NyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAxMC0xMS0yNiAgUGF2ZWwgRmVsZG1h biAgPHBmZWxkbWFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBXZWIgSW5zcGVjdG9yOiBjcmFzaCB1cG9uIGluc3BlY3Rpbmcg dXNlciBzdHlsZSBzaGVldC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTUwMTA5CisKKyAgICAgICAgKiBpbnNwZWN0b3IvSW5zcGVjdG9yU3R5bGVTaGVl dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpJbnNwZWN0b3JTdHlsZVNoZWV0OjpyZXNvdXJjZVN0 eWxlU2hlZXRUZXh0KToKKwogMjAxMC0xMS0yNSAgUGF2ZWwgRmVsZG1hbiAgPHBmZWxkbWFuQGNo cm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBZdXJ5IFNlbWlraGF0c2t5LgpkaWZm IC0tZ2l0IGEvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yU3R5bGVTaGVldC5jcHAgYi9XZWJD b3JlL2luc3BlY3Rvci9JbnNwZWN0b3JTdHlsZVNoZWV0LmNwcAppbmRleCAwYWQzODIxLi5lYzFi YjM5IDEwMDY0NAotLS0gYS9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JTdHlsZVNoZWV0LmNw cAorKysgYi9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JTdHlsZVNoZWV0LmNwcApAQCAtOTg2 LDYgKzk4Niw5IEBAIGJvb2wgSW5zcGVjdG9yU3R5bGVTaGVldDo6b3JpZ2luYWxTdHlsZVNoZWV0 VGV4dChTdHJpbmcqIHJlc3VsdCkgY29uc3QKIAogYm9vbCBJbnNwZWN0b3JTdHlsZVNoZWV0Ojpy ZXNvdXJjZVN0eWxlU2hlZXRUZXh0KFN0cmluZyogcmVzdWx0KSBjb25zdAogeworICAgIGlmICht X29yaWdpbiA9PSAidXNlciIgfHwgbV9vcmlnaW4gPT0gInVzZXItYWdlbnQiKQorICAgICAgICBy ZXR1cm4gZmFsc2U7CisKICAgICBpZiAoIW1fcGFnZVN0eWxlU2hlZXQgfHwgIW93bmVyRG9jdW1l bnQoKSkKICAgICAgICAgcmV0dXJuIGZhbHNlOwogCg==