Bug 26985

Summary: Implement parsing of <rp> & <rt> elements as per HTML5 spec
Product: WebKit Reporter: Roland Steiner <rolandsteiner>
Component: DOMAssignee: Roland Steiner <rolandsteiner>
Status: RESOLVED FIXED    
Severity: Enhancement CC: bfulgham, mike, mjs
Priority: P3 Keywords: HTML5
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch: implement parsing of <rt>/<rp>, add accessibility role
mjs: review-
patch: update with layout test mjs: review+

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