Bug 58984 - Replace bidi-004.html with its new version.
Summary: Replace bidi-004.html with its new version.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-20 06:39 PDT by Yael
Modified: 2011-04-20 12:28 PDT (History)
8 users (show)

See Also:


Attachments
Patch. (113.75 KB, patch)
2011-04-20 06:44 PDT, Yael
no flags Details | Formatted Diff | Diff
Patch. (114.32 KB, patch)
2011-04-20 06:48 PDT, Yael
no flags Details | Formatted Diff | Diff
Patch. (113.75 KB, patch)
2011-04-20 07:00 PDT, Yael
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yael 2011-04-20 06:39:15 PDT
The test bidi-004.html was removed yesterday from the CSS 2.1 test suite, as it was deemed wrong.
WebKit should have the new version of the test, so that we don't get more comments like https://bugs.webkit.org/show_bug.cgi?id=57459#c25.
Comment 1 Yael 2011-04-20 06:44:56 PDT
Created attachment 90335 [details]
Patch.
Comment 2 Yael 2011-04-20 06:48:56 PDT
Created attachment 90336 [details]
Patch.

One more try :)
Comment 3 Yael 2011-04-20 07:00:18 PDT
Created attachment 90337 [details]
Patch.
Comment 4 Yael 2011-04-20 07:19:21 PDT
Comment on attachment 90337 [details]
Patch.

Not sure why removing images is causing a problem.
Will commit manually after review.
Comment 5 Eric Seidel (no email) 2011-04-20 07:58:36 PDT
Comment on attachment 90337 [details]
Patch.

Your local checkout is not correctly attributing pngs as binary.
Comment 6 Eric Seidel (no email) 2011-04-20 07:59:12 PDT
(At least I believe that's what happened to your diff.)
Comment 7 Eric Seidel (no email) 2011-04-20 08:06:16 PDT
I'm a bit confused with all this.  Can you point me to the decision where bidi-04 was deemed wrong?  Can you explain what your new bidi-04 does?
Comment 8 Yael 2011-04-20 08:21:30 PDT
(In reply to comment #7)
> I'm a bit confused with all this.  Can you point me to the decision where bidi-04 was deemed wrong?  Can you explain what your new bidi-04 does?

After a discussion on IRC last night, Hixie said he was going to send an e-mail to www-style about bidi-004.xht and bidi-004a.xht being wrong. I do not see that e-mail, so it probably did not happen yet.

I created this bug based on Hixie's comment and the test he created in http://www.hixie.ch/tests/adhoc/css/box/inline/bidi/012.html .
Comment 9 Yael 2011-04-20 08:22:51 PDT
(In reply to comment #5)
> (From update of attachment 90337 [details])
> Your local checkout is not correctly attributing pngs as binary.

I set svn propset svn:mime-type on these files, but that did not help.
Anything else I should do?
thanks,
Comment 10 Levi Weintraub 2011-04-20 10:56:14 PDT
(In reply to comment #7)
> I'm a bit confused with all this.  Can you point me to the decision where bidi-04 was deemed wrong?  Can you explain what your new bidi-04 does?

Yael and I talked to Hixie on IRC and discussed the change I had made to treat BR's as paragraph separators that don't break out of style/dom directionality, but only unicode control characters. That change prevented us from passing the old bidi-004.html test, and after showing it to him, he concurred that in fact the test was flawed.
Comment 11 Eric Seidel (no email) 2011-04-20 11:05:59 PDT
Comment on attachment 90337 [details]
Patch.

It may not be your diff, it may be prettypatch which is causing the strageness.
Comment 12 Yael 2011-04-20 11:16:53 PDT
Committed r84398: <http://trac.webkit.org/changeset/84398>
Comment 13 WebKit Review Bot 2011-04-20 12:28:11 PDT
http://trac.webkit.org/changeset/84398 might have broken Windows 7 Release (Tests)
The following tests are not passing:
fast/borders/bidi-012.html