Bug 119963 – Use TextNodeTraversal for getting sheet text in StyleElement

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Antti Koivisto]

foo

[Antti Koivisto]

Created attachment 209012 [details]
oatch

[Andreas Kling]

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

[Antti Koivisto]

https://trac.webkit.org/r154242

[Antti Koivisto]

Created attachment 209022 [details]
remove now crashing test

[Andreas Kling]

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().

[Andreas Kling]

Comment on attachment 209022 [details]
remove now crashing test

We should just expect this test to [ Crash ] for now.

[Antti Koivisto]

Filed bug 119976 for that.

[Antti Koivisto]

Created attachment 209023 [details]
mark as Crash

[Antti Koivisto]

https://trac.webkit.org/r154246 for TestExpectations

[Darin Adler]

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.

[Ryosuke Niwa]

CRASH expectation in this case doesn't work well in that the test sometimes times off, trying to allocate a very large memory space.

[Ryosuke Niwa]

Marked the test slow in http://trac.webkit.org/changeset/154352.

[Antti Koivisto]

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.

[Ryosuke Niwa]

(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.

[Ryosuke Niwa]

Filed webkit.org/b/120095 to remove the test.