Bug 47153

Created attachment 74501 [details] Patch

This fixes the specified tests, let me know if tests need to be added or whether the harness is enough. Cheers, Rob.

Created attachment 74506 [details] Added tests and really fixed 09

Comment on attachment 74506 [details] Added tests and really fixed 09 View in context: https://bugs.webkit.org/attachment.cgi?id=74506&action=review These tests are strange. Somehow, a syntax requirement of @import being the first rule (besides @charset) can be waived by semantical correctness of preceding rules - I wouldn't expect it to work that way. r=me, but please don't land without confirming that IE and Firefox agree. > WebCore/ChangeLog:8 > + Fix at-import-009.htm by ignoring invalid @media and @charset rules. Valid or not, @charset is allowed before @import. Could you add an explanation of what the problem with that was?

Hi Alexey, Sorry to react so late, very busy with other things lately. (In reply to comment #4) > (From update of attachment 74506 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=74506&action=review > > These tests are strange. Somehow, a syntax requirement of @import being the first rule (besides @charset) can be waived by semantical correctness of preceding rules - I wouldn't expect it to work that way. > > r=me, but please don't land without confirming that IE and Firefox agree. Indeed at-import-009.htm works in FF but fails in IE. at-import-010.htm works as expected in both (text shows up green). If at-import-009.htm as a test is questionable, should I redo the patch to only fix at-import-010.htm? Cheers, Rob.

Sounds good. I'd suggest contacting test suite maintainer about these tests - perhaps this has all been considered already.

Discussion of the test suite happens on public-css-testsuite@w3.org.

Created attachment 75313 [details] Fix html4/at-import-010 test

Comment on attachment 75313 [details] Fix html4/at-import-010 test Somehow, I expected this test to be more controversial, because ":unknownpseudo { background: red; }" seems "more valid" than "@page". Weird. This test doesn't contain any page rules, but you modify CSSParser::createPageRule(). r=me assuming skipping invalid @page rules matches both IE and Firefox. It's unfortunate to land this without a test.

Hello Alexey, Simon, (In reply to comment #9) > (From update of attachment 75313 [details]) > Somehow, I expected this test to be more controversial, because ":unknownpseudo { background: red; }" seems "more valid" than "@page". Weird. > > This test doesn't contain any page rules, but you modify CSSParser::createPageRule(). > > r=me assuming skipping invalid @page rules matches both IE and Firefox. It's unfortunate to land this without a test. I decided to skip the @page bits as it was not needed for fixing html4/at-import-010. That part of the patch is now in webkit repository. I noticed later that at-import-009.htm does actually work in IE! I can only guess that I forgot to create the support/import-green.css on my windows XP intially, but again after a later retry it worked. So should I create a new patch for at-import-009.htm fixing or should I first ask about its validity on public-css-testsuite@w3.org? Cheers, Rob.

Well, if these tests pass in both IE and Firefox, then we should just proceed.

Attachment 75313 [details] was posted by a committer and has review+, assigning to Rob Buis for commit.

Hi Eric, (In reply to comment #12) > Attachment 75313 [details] was posted by a committer and has review+, assigning to Rob Buis for commit. Sorry for my tardiness here, for one thing I was travelling lately. The other, more serious thing, is that last week before landing I decided to recheck and actually found FF4 on OS X failing the test! So maybe we have to wait landing this and clear up validness of the testcase on the css tests mailing list? Cheers, Rob.

Comment on attachment 75313 [details] Fix html4/at-import-010 test Clearing review flag while we wait to hear if the test is valid or not.

Created attachment 460293 [details] Safari 15.5 matches other browsers I am unable to reproduce this bug in Safari 15.5 on macOS 12.4 based on the following test case: Test - http://test.csswg.org/suites/css21_dev/20110323/html4/at-import-010.htm From Comment 05, it seems that 009 was questionable and the later bug was focused on fixing alt-import-010 but didn't move due to test being valid as per Comment 14. Since now all browsers as shown in the picture are working as expected and showing "Green" text, I think this can be marked as "RESOLVED INVALID" or "RESOLVED CONFIGURATION CHANGED". Depending on, whether it was web-spec change, which aligned behavior or something along the line got it fixed. Thanks!