Bug 48593 - HTML5 Conformance Test failure: approved/xhtml5/html_style_in_comment.xhtml
Summary: HTML5 Conformance Test failure: approved/xhtml5/html_style_in_comment.xhtml
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tony Gentilcore
URL: http://test.w3.org/html/tests/approve...
Keywords:
Depends on:
Blocks: 48575
  Show dependency tree / graph
 
Reported: 2010-10-28 16:48 PDT by David Kilzer (:ddkilzer)
Modified: 2011-01-08 17:31 PST (History)
6 users (show)

See Also:

Attachments
Patch (6.02 KB, patch)
2011-01-07 14:31 PST, Tony Gentilcore 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2010-10-28 16:48:34 PDT 
The approved/xhtml5/html_style_in_comment.xhtml test fails with WebKit nightly build r70732 run in Safari 5.0.x.
Comment 1 Tony Gentilcore 2011-01-06 14:18:11 PST 
Yeah, we don't match IE9 and FF4. I'll take a look.
Comment 2 Tony Gentilcore 2011-01-06 16:40:49 PST 
Does it makes sense to import these tests directly (similar to what we did with html5lib)?

I'm thinking about adding only this test into a new directory with this patch:
LayoutTests/w3html/xhtml5/html_style_in_comment.xhtml

The idea being that LayoutTests/w3html/ would house the same structure as http://test.w3.org/html/tests/approved/
Comment 3 Adam Barth 2011-01-06 16:46:48 PST 
Yeah, we've historically imported these test suites when the license allowed for it.
Comment 4 Tony Gentilcore 2011-01-06 16:55:55 PST 
(In reply to comment #3)
> Yeah, we've historically imported these test suites when the license allowed for it.

OTOH, this particular test kind of sucks because rather than displaying PASS or FAIL, it simply changes the color of some text. That forces it to be a pixel test instead of using dumpAsText().

I'm wondering if it wouldn't be better to just rewrite so they don't have to be pixel tests. :-(
Comment 5 Adam Barth 2011-01-06 17:40:03 PST 
Can we have a harness that looks inside and reads the computed style?
Comment 6 Tony Gentilcore 2011-01-07 14:31:41 PST 
Created attachment 78271 [details]
Patch
Comment 7 Tony Gentilcore 2011-01-07 14:35:00 PST 
(In reply to comment #5)
> Can we have a harness that looks inside and reads the computed style?

After looking over the other tests in the suite, I decided to just go for an ad hoc test in this patch. Most of the tests don't seem to lend themselves well to our style of automated testing and it will be a slightly more involved task to come up with a scheme for running them.
Comment 8 Eric Seidel (no email) 2011-01-07 14:51:48 PST 
Comment on attachment 78271 [details]
Patch

Wow.  Crazy.
Comment 9 WebKit Commit Bot 2011-01-08 17:31:25 PST 
Comment on attachment 78271 [details]
Patch

Clearing flags on attachment: 78271

Committed r75338: <http://trac.webkit.org/changeset/75338>
Comment 10 WebKit Commit Bot 2011-01-08 17:31:33 PST 
All reviewed patches have been landed.  Closing bug.