Summary: | ASSERTION FAILED: name[0] == '@' && length >= 2 in WebCore::CSSParser::detectAtToken | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Martin Hodovan <mhodovan.u-szeged> | ||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, darin, esprehn+autocc, ggaren, glenn, gyuyoung.kim, macpherson, menard, msaboff, rhodovan.u-szeged, rniwa, selenagomezzrw, simon.fraser | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 116980 | ||||||||||||
Attachments: |
|
Description
Martin Hodovan
2014-07-04 06:02:26 PDT
Created attachment 234405 [details] Proposed patch Backported from Chromium: https://codereview.chromium.org/241053002 Comment on attachment 234405 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=234405&action=review > Source/WebCore/css/CSSParser.cpp:11231 > + // The standard enables unicode escapes in at-rules. In this case only the resultString will contain the > + // correct identifier, hence we have to use it to determine its length instead of the usual pointer arithmetic. The bug is in parseIdentifier, which needs to bump the result pointer even in the 16-bit slow case. Not doing that will cause many other problems, so just changing this code path to quiet the assertion is wrong. The line of code that should be added to parseIdentifer is: result += result16 - start16; The reason it’s important to fix the bug in parseIdentifier is that multiple call sites of parseIdentifier are affected by this, not just this one code path. But also, the 16-bit code path in parseIdentifier doesn’t really need to switch to parsing 16-bit input. It’s only the output string that needs to be 16-bit, and the way it currently does things is unnecessarily inefficient. This code is made terribly confusing by using the name "result" for a pointer to the next character to be parsed. We should call this current, not result, and we should also investigate using a const pointer for it and eliminating code that writes into it. But that’s beyond the scope of this. Created attachment 235687 [details]
Proposed patch
Thank you for the complete solution and sorry about the delay. I updated the patch.
(I am making a follow-up patch to rename the confusing 'result' pointer to 'current'.)
Comment on attachment 235687 [details] Proposed patch Attachment 235687 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5421317947916288 New failing tests: media/media-fragments/TC0001.html Created attachment 235697 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 236025 [details]
Proposed patch
Comment on attachment 236025 [details] Proposed patch Clearing flags on attachment: 236025 Committed r172036: <http://trac.webkit.org/changeset/172036> All reviewed patches have been landed. Closing bug. |