Bug 26985 - Implement parsing of <rp> & <rt> elements as per HTML5 spec
Summary: Implement parsing of <rp> & <rt> elements as per HTML5 spec
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Enhancement
Assignee: Roland Steiner
URL:
Keywords: HTML5
Depends on:
Blocks:
 
Reported: 2009-07-05 23:30 PDT by Roland Steiner
Modified: 2009-07-18 05:45 PDT (History)
3 users (show)

See Also:


Attachments
patch: implement parsing of <rt>/<rp>, add accessibility role (6.61 KB, patch)
2009-07-05 23:34 PDT, Roland Steiner
mjs: review-
Details | Formatted Diff | Diff
patch: update with layout test (9.16 KB, patch)
2009-07-06 03:20 PDT, Roland Steiner
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Steiner 2009-07-05 23:30:41 PDT
The HTML5 spec specifies that end tags of the ruby tags <rp> and <rt> are optional if followed by another <rp> or <rt> tag:

http://www.whatwg.org/specs/web-apps/current-work/#optional-tags
Comment 1 Roland Steiner 2009-07-05 23:34:45 PDT
Created attachment 32286 [details]
patch: implement parsing of <rt>/<rp>, add accessibility role

This patch implements the parsing of <rp> and <rt> tags as per the HTML5 spec. 

It also sets a (new) accessibility role "Annotation" for <rp> and <rt>.

Affected code parts are not enclosed in #IF ENABLE(RUBY), since the parsing is not affected by whether ruby is rendered properly or not (in fact, it may be more profound without ruby layouting, since the contents of <rp> are not hidden).

The patch includes no layout tests, as layouting will change once the patch for proper ruby rendering is landed.
Comment 2 Maciej Stachowiak 2009-07-06 00:25:49 PDT
(In reply to comment #1)
> Created an attachment (id=32286) [details]
> patch: implement parsing of <rt>/<rp>, add accessibility role
> 
> This patch implements the parsing of <rp> and <rt> tags as per the HTML5 spec. 
> 
> It also sets a (new) accessibility role "Annotation" for <rp> and <rt>.
> 
> Affected code parts are not enclosed in #IF ENABLE(RUBY), since the parsing is
> not affected by whether ruby is rendered properly or not (in fact, it may be
> more profound without ruby layouting, since the contents of <rp> are not
> hidden).
> 
> The patch includes no layout tests, as layouting will change once the patch for
> proper ruby rendering is landed.

I think this patch needs regression tests anyway, since it affects behavior. Either the tests should be made so they only test parsing and not layout, or the results can be updated when layout changes too.
Comment 3 Maciej Stachowiak 2009-07-06 00:27:16 PDT
Comment on attachment 32286 [details]
patch: implement parsing of <rt>/<rp>, add accessibility role

The code change looks good. However, I am going to say review- due to lack of tests for the newly introduced parsing behavior.
Comment 4 Roland Steiner 2009-07-06 03:20:34 PDT
Created attachment 32296 [details]
patch: update with layout test

Adding a layout test was a good thing because it turns out that for proper parsing I also needed to add code for the outer <ruby> tag. Without this, <rp>/<rt> wouldn't be automatically closed if no end tag is present before </ruby>.
Comment 5 Maciej Stachowiak 2009-07-09 20:15:35 PDT
Comment on attachment 32296 [details]
patch: update with layout test

r=me
Comment 6 Brent Fulgham 2009-07-09 21:43:19 PDT
Landed in http://trac.webkit.org/changeset/45698