New ruby parsing rule was implemented at r167437. Gecko also implemented it at [1]. After landing in Gecko, we found that some real web sites were broke by the change[2]. Although the broken site[3] used invalid markup like "<rtc><rp>(</rp>foo</rtc>", we should not break the real web sites. Then, spec change was requested to W3C[4], and HTML5.1 nightly spec was changed[5]. We should follow the spec change not to break the web. [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=664104 [2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1042885 [3]: http://ikilote.net/fr/Collection/Personnalit%C3%A9.html?id=394 [4]: https://www.w3.org/Bugs/Public/show_bug.cgi?id=26424 [5]: https://github.com/w3c/html/commit/c61397b989b28235ee2228f280aa8d475f3b9ebf
Created attachment 236803 [details] Patch
This patch didn't fix layout problem at http://ikilote.net/fr/Collection/Personnalit%C3%A9.html?id=394 I think this is another problem. DOM looks identical to the DOM of Gecko or old Safari. The reason why WebKit has layout problem is that WebKit supports @support, but Safari 7.0.3 doesn't support it. When @support is supported and display: ruby is not supported, the site try to simulate display: ruby behavior using display: table to ruby. Since WebKit doesn't support ruby other than display: block and inline, the layout becomes strange. However, I'm not sure why "ruby.ruby_h > rtc { display: table-header-group;}" is not applied to the rtc element. Gecko apply the style.
It looks like WebKit does not like :not() selector; see this file: http://jsbin.com/nerece/1/edit
(In reply to comment #3) > It looks like WebKit does not like :not() selector; see this file: http://jsbin.com/nerece/1/edit Thank you for investigating. I also think this is problem.
Narrowed down and entered bug 136063. More specifically, not() selector stops working if within or after @supports, only in Nightly build.
Comment on attachment 236803 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236803&action=review Thanks for the patch but we need a new test case demonstrating the issue reported to Gecko. r- due to the lack of tests. > Source/WebCore/ChangeLog:11 > + The HTML5 spec was changed at the following commit: > + https://github.com/w3c/html/commit/c61397b989b28235ee2228f280aa8d475f3b9ebf It's great to see the hyperlink to the relevant spec change! > Source/WebCore/ChangeLog:14 > + This patch changed the RP element's behavior to follow the spec change. > + We should add a new test. It would be great if we could use the problem reported at https://bugzilla.mozilla.org/show_bug.cgi?id=1042885.
Created attachment 236816 [details] Minimized test case from http://ikilote.net/fr/Collection/Personnalité.html?id=394 for bug 136062 (In reply to comment #6) > > Source/WebCore/ChangeLog:14 > > + This patch changed the RP element's behavior to follow the spec change. > > + > > We should add a new test. > It would be great if we could use the problem reported at https://bugzilla.mozilla.org/show_bug.cgi?id=1042885. Yuki, attached is the minimized test case from the page the problem was reported.
Created attachment 236850 [details] Patch
Hi Ryosuke, (In reply to comment #6) > (From update of attachment 236803 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=236803&action=review > > Thanks for the patch but we need a new test case demonstrating the issue reported to Gecko. > r- due to the lack of tests. > > > Source/WebCore/ChangeLog:11 > > + The HTML5 spec was changed at the following commit: > > + https://github.com/w3c/html/commit/c61397b989b28235ee2228f280aa8d475f3b9ebf > > It's great to see the hyperlink to the relevant spec change! Added. But, I cannot find more closer link. > > > Source/WebCore/ChangeLog:14 > > + This patch changed the RP element's behavior to follow the spec change. > > + > > We should add a new test. > It would be great if we could use the problem reported at https://bugzilla.mozilla.org/show_bug.cgi?id=1042885. Added test case based on Koji's minimized test case.
For the record, IMHO this whole exercise is a bad idea. We shouldn't be using the W3C's newfangled ruby rules; they are way more complicated than necessary and serve no useful use cases beyond what the much simpler WHATWG model serves.
(In reply to comment #10) > For the record, IMHO this whole exercise is a bad idea. We shouldn't be using the W3C's newfangled ruby rules; they are way more complicated than necessary and serve no useful use cases beyond what the much simpler WHATWG model serves. rt element is used by real Web content.
(In reply to comment #11) > (In reply to comment #10) > > For the record, IMHO this whole exercise is a bad idea. We shouldn't be using the W3C's newfangled ruby rules; they are way more complicated than necessary and serve no useful use cases beyond what the much simpler WHATWG model serves. > > rt element is used by real Web content. I meant rb.
Comment on attachment 236850 [details] Patch Clearing flags on attachment: 236850 Committed r172834: <http://trac.webkit.org/changeset/172834>
All reviewed patches have been landed. Closing bug.
This broke fast/ruby/ruby-base-merge-block-children-crash-2.html. Given the breakage, and the comments from Hixie, I think we should roll this out.
(In reply to comment #15) > This broke fast/ruby/ruby-base-merge-block-children-crash-2.html. Given the breakage, and the comments from Hixie, I think we should roll this out. Sorry for breaking the test. It's a bit strange because this patch only change a rp element behavior, and the test doesn't contain rp element. EWS and my local build didn't fail the test. I updated repo and am building WebKit. I try to reproduce it locally.
This is not regression of this bug. The regression was introduced by the following commit: http://trac.webkit.org/changeset/172835 The reason why the test fails is the commit updated test but not updated text expectation.
I created the patch to update the expectation at Bug 136144 .
Thank you! Apologies for blaming this patch.
rniwa: What real content using <rb> breaks when parsed as per the WHATWG parsing rules? Can you file a bug on the WHATWG spec if that's the case? http://whatwg.org/newbug