Bug 28420 - Implement HTML5 <ruby> rendering
: Implement HTML5 <ruby> rendering
Status: RESOLVED FIXED
: WebKit
Layout and Rendering
: 528+ (Nightly build)
: All All
: P3 Normal
Assigned To:
:
:
:
: 32934
  Show dependency treegraph
 
Reported: 2009-08-17 22:15 PST by
Modified: 2010-01-25 01:05 PST (History)


Attachments
patch 1: remove ENABLE_RUBY flag as discussed with Dave and Maciej (31.15 KB, patch)
2009-08-17 23:22 PST, Roland Steiner
eric: review+
commit-queue: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
patch 2: ruby functionality (54.72 KB, patch)
2009-08-18 03:25 PST, Roland Steiner
no flags Review Patch | Details | Formatted Diff | Diff
patch 3: layout tests for ruby (713.83 KB, patch)
2009-08-18 03:26 PST, Roland Steiner
hyatt: review+
commit-queue: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
patch 2: ruby functionality (54.26 KB, patch)
2009-08-24 22:31 PST, Roland Steiner
hyatt: review+
commit-queue: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
patch 4 - re-baselined layout test results for Windows (675.77 KB, patch)
2009-11-04 04:03 PST, Roland Steiner
eric: review+
Review Patch | Details | Formatted Diff | Diff
patch 5a - remove fast/ruby tests from Skipped list for Mac (1.15 KB, patch)
2009-11-11 21:07 PST, Roland Steiner
darin: review+
Review Patch | Details | Formatted Diff | Diff
patch 5b - remove fast/ruby tests from Skipped list for Win (1.61 KB, patch)
2010-01-20 23:34 PST, Roland Steiner
mjs: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-08-17 22:15:50 PST
Implement proper rendering for <ruby>, <rt> and <rp> elements as per the HTML5 spec.

See http://www.whatwg.org/specs/web-apps/current-work/#the-ruby-element .

This bug is similar to https://bugs.webkit.org/show_bug.cgi?id=3749, but focuses on a HTML5-based implementation rather than a CSS3/XHTML-based implementation.
------- Comment #1 From 2009-08-17 23:22:22 PST -------
Created an attachment (id=35020) [details]
patch 1: remove ENABLE_RUBY flag as discussed with Dave and Maciej

Remove ENABLE_RUBY guards as discussed with Dave Hyatt and Maciej Stachowiak on IRC.
------- Comment #2 From 2009-08-18 03:25:03 PST -------
Created an attachment (id=35033) [details]
patch 2: ruby functionality

Fundamentally the same patch as the last one submitted to https://bugs.webkit.org/show_bug.cgi?id=3749 .

See the descriptions there for discussions and caveats (brief summary: implements HTML5 ruby only, no CSS3/XHTML support, some minor known issues with selection and copy/paste).
------- Comment #3 From 2009-08-18 03:26:35 PST -------
Created an attachment (id=35034) [details]
patch 3: layout tests for ruby

New layout tests for <ruby>. These test 2 main areas:

.) basic rendering
.) correctness of render tree after DOM manipulation
------- Comment #4 From 2009-08-24 22:31:02 PST -------
Created an attachment (id=38526) [details]
patch 2: ruby functionality

Updated patch: first version attempted to be independent of other patches included here. However, this was not feasible and would have resulted in problems when committing. Also improved the change log entry.

Provided these patches are accepted, they should be committed in numbered order, i.e., "patch 1" first, then "patch 2", and "patch 3" last.
------- Comment #5 From 2009-08-25 05:43:32 PST -------
Rebaselined versions of the layout tests for Chromium-Win:

http://codereview.chromium.org/173347
------- Comment #6 From 2009-09-03 01:38:07 PST -------
(From update of attachment 35020 [details])
I'll trust you that they agreed to this.  This looks technically correct.
------- Comment #7 From 2009-09-03 01:39:08 PST -------
(From update of attachment 35020 [details])
Actually, removing cq+, since the commit-queue will close the bug when it lands this patch (despite the other r=? patches) due to a bug.
------- Comment #8 From 2009-09-03 01:39:42 PST -------
bug 28230 is the bug I was referring to.
------- Comment #9 From 2009-09-29 11:22:49 PST -------
(From update of attachment 38526 [details])
Typo in RenderRubyAsInline:

"Note: ':after' content is handled implicitely below"

"implicitely" hould be "implicitly"

Everything else looks good.

r=me
------- Comment #10 From 2009-10-19 08:50:00 PST -------
(From update of attachment 35020 [details])
Let commit bot land it
------- Comment #11 From 2009-10-19 09:27:14 PST -------
(From update of attachment 35020 [details])
Rejecting patch 35020 from commit-queue.

Patch https://bugs.webkit.org/attachment.cgi?id=35020 from bug 28420 failed to download and apply.
------- Comment #12 From 2009-10-19 09:32:15 PST -------
(From update of attachment 35034 [details])
Rejecting patch 35034 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11484 test cases.
fast/ruby/ruby-empty-rt.html -> failed

Exiting early after 1 failures. 7830 tests run.
141.56s total testing time

7829 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
3 test cases (<1%) had stderr output
------- Comment #13 From 2009-10-19 09:32:49 PST -------
(From update of attachment 38526 [details])
Rejecting patch 38526 from commit-queue.

Patch https://bugs.webkit.org/attachment.cgi?id=38526 from bug 28420 failed to download and apply.
------- Comment #14 From 2009-10-19 15:15:28 PST -------
The patches depend on each other and probably need someone to land them manually.  The commit-queue isn't really designed for this type of multi-patch change.
------- Comment #15 From 2009-10-19 19:33:41 PST -------
(In reply to comment #14)
> The patches depend on each other and probably need someone to land them
> manually.  The commit-queue isn't really designed for this type of multi-patch
> change.

Thanks for handling this so far! I'll get my committer account for WebKit soon, so I should be able to land it myself then.
------- Comment #16 From 2009-10-30 02:17:17 PST -------
Landed patch 1.
------- Comment #17 From 2009-11-01 19:17:13 PST -------
Landed patch 2.
------- Comment #18 From 2009-11-01 21:04:02 PST -------
http://trac.webkit.org/changeset/50397 broke the windows build. We need to fix it or roll it out.
------- Comment #19 From 2009-11-01 21:19:59 PST -------
Fixed the windows build.
------- Comment #20 From 2009-11-04 04:03:17 PST -------
Created an attachment (id=42475) [details]
patch 4 - re-baselined layout test results for Windows

Added new patch that contains the re-baselined results for the ruby layout tests for the Windows platform.
------- Comment #21 From 2009-11-04 19:59:14 PST -------
Landed patch 3

As it caused another layout test to fail, the ruby layout tests are currently added to the Skipped list for Mac.

After more investigation it turns out that the fault seems to lay with the single failing layout test, however.
------- Comment #22 From 2009-11-06 11:29:42 PST -------
*** Bug 3749 has been marked as a duplicate of this bug. ***
------- Comment #23 From 2009-11-10 22:20:38 PST -------
Can we remove the tests from the Windows Skipped List now?
------- Comment #24 From 2009-11-11 10:45:39 PST -------
I tried re-running these tests with the new Windows specific results, and they still failed on my machine. Any idea why that would be happening?
------- Comment #25 From 2009-11-11 16:23:28 PST -------
(In reply to comment #24)
> I tried re-running these tests with the new Windows specific results, and they
> still failed on my machine. Any idea why that would be happening?

Not really :p - I generated the re-baselined results on an XP laptop, but perhaps something in the setup wasn't quite correct. Is there a way to have these auto-generated instead?
------- Comment #26 From 2009-11-11 21:07:56 PST -------
Created an attachment (id=43029) [details]
patch 5a - remove fast/ruby tests from Skipped list for Mac

After https://bugs.webkit.org/show_bug.cgi?id=31200 has been fixed, there is no reason to skip the ruby layout tests for Mac any longer.
------- Comment #27 From 2009-12-02 12:17:56 PST -------
Roland: Ping?
------- Comment #28 From 2009-12-02 19:30:02 PST -------
(In reply to comment #27)
> Roland: Ping?

Yes, sorry, I should have posted an update here too: I am currently re-writing the layout tests to use latin characters - see https://bugs.webkit.org/show_bug.cgi?id=31865.

I want to submit the baselines for both Mac and Windows there before re-enabling them, to avoid having some intermediate state between old and new versions.
------- Comment #29 From 2009-12-14 14:04:19 PST -------
Should we r- these patches then?  I'm unclear on what should be done with this bug.
------- Comment #30 From 2009-12-14 18:13:50 PST -------
(In reply to comment #29)
Sorry, I should have been more clear in my updates - patch 4 is indeed obsolete once I (finally) get to do the Windows baselines of the new Latin-only versions.

As for patch 5a, that's basically still valid, I just wanted to get the baselines in before enabling the layout tests, followed by a "patch 5b" to also enable them for Windows.

However, in order to avoid confusion, it may be easier to r- patch 5a here. I'll just re-add it to https://bugs.webkit.org/show_bug.cgi?id=31865 once I get around with the Windows baselines there (or open a new bug entry to enable both Mac and Windows layout tests).
------- Comment #31 From 2010-01-20 00:43:28 PST -------
(From update of attachment 42475 [details])
Mark patch 4 as obsolete.
------- Comment #32 From 2010-01-20 03:34:21 PST -------
Landed patch 5a as rev. 53531
------- Comment #33 From 2010-01-20 23:34:38 PST -------
Created an attachment (id=47101) [details]
patch 5b - remove fast/ruby tests from Skipped list for Win

Last patch for full first implementation of ruby: remove ruby layout tests from Skipped list for Windows.

After the landing of the new Latin-character only layout tests (https://bugs.webkit.org/show_bug.cgi?id=31865), and the resolving of the issue that caused other tests to fail (not actually caused by the ruby layout tests(cf. https://bugs.webkit.org/show_bug.cgi?id=31200, problem not actually caused by ruby), there's no reason to leave the ruby layout tests in the Skipped list for Windows.
------- Comment #34 From 2010-01-21 00:49:39 PST -------
(From update of attachment 47101 [details])
r=me

In the future, please try to make new bugs for new patches instead of reusing the same bug - it's confusing when multiple independent changes are associated with the same bug. And webkit-patch makes it super easy to file a new bug for each patch.
------- Comment #35 From 2010-01-25 01:05:54 PST -------
Patch 5b landed as 53791, with follow-up fix in 53793