Summary: | Use TextNodeTraversal for getting sheet text in StyleElement | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||||||
Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, esprehn+autocc, kangil.han, rniwa | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Antti Koivisto
2013-08-17 15:27:39 PDT
Created attachment 209012 [details]
oatch
Comment on attachment 209012 [details] oatch View in context: https://bugs.webkit.org/attachment.cgi?id=209012&action=review Gorgeous! > Source/WebCore/ChangeLog:11 > + (which is used by contentsAsString) does that itself. The behavior in case of overflow chances from empty chances -> changes Created attachment 209022 [details]
remove now crashing test
Comment on attachment 209022 [details]
remove now crashing test
I think we should have a way for layout tests to PASS when doing a controlled CRASH().
Comment on attachment 209022 [details]
remove now crashing test
We should just expect this test to [ Crash ] for now.
Filed bug 119976 for that. Created attachment 209023 [details]
mark as Crash
https://trac.webkit.org/r154246 for TestExpectations Comment on attachment 209012 [details] oatch View in context: https://bugs.webkit.org/attachment.cgi?id=209012&action=review > Source/WebCore/dom/StyleElement.cpp:114 > + String sheetText = TextNodeTraversal::contentsAsString(element); > > - createSheet(e, m_startLineNumber, sheetText.toString()); > + createSheet(element, m_startLineNumber, sheetText); For me this would read even better without a local variable. CRASH expectation in this case doesn't work well in that the test sometimes times off, trying to allocate a very large memory space. Marked the test slow in http://trac.webkit.org/changeset/154352. Maybe we should just get rid of the test? There are many ways you can allocate unreasonable amounts of memory and it is bit silly to be testing this particular one. (In reply to comment #13) > Maybe we should just get rid of the test? There are many ways you can allocate unreasonable amounts of memory and it is bit silly to be testing this particular one. That might make sense after all. Filed webkit.org/b/120095 to remove the test. |