1084183 - Propagate text decoration to ruby frames

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Xidorn Quan [:xidorn] UTC+11]

Currently, text decorations are not propagated into ruby frames.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch

Currently the lines generated by text decoration are not continuous. It is because the width of text frames is not expanded properly, which should be solved in bug 1055676.

Comment 2

[David Baron :dbaron:]

Comment on attachment 8507607 [details] [diff] [review]
patch

r=dbaron on the code, but it would be good to add a test, and make sure it's specified clearly in the css-ruby spec.  Ideally, the test would go in layout/reftests/w3c-css/submitted/css-ruby/ so that we submit it to the CSS WG as well.

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

Actually, I guess this test should go in text-decor-3/ instead of css-ruby/, because it is not part of ruby spec, but of Text Decoration 3: http://dev.w3.org/csswg/css-text-decor-3/#line-decoration

> When specified on or propagated to a ruby box, the decorations are propagated only to the ruby base.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

I forgot that I didn't write test for it. Remove checkin-needed.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch of test

It is strange that, in a normal browser the test page and the reference page look same, but when running in reftest, they are different. The rendering result of test page is not understandable. I cannot figure out the reason.

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch of test

Comment 7

[David Baron :dbaron:]

Comment on attachment 8513896 [details] [diff] [review]
patch of test

r=dbaron

It might also make sense to have a second reference with a span inside the rb, and the text-decoration on the span.

Also, I think I'd prefer the directory be called either text-decor-3 or text-decor.

You should remove the link rel="help" from the reference.

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

My patch 1 of bug 1088489 causes some problems in the test for this bug. The problems will be fixed by the patch 2.

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch of test

Changes:
1. The directory name is changed to text-decor-3
2. Wrap content in <rb> with <span>
3. Remove inter-segment whitespaces

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ddc8963838ae

Comment 11

[David Baron :dbaron:]

Comment on attachment 8523482 [details] [diff] [review]
patch of test

Please put the link rel="help" back in the test.  (That is, it should be in the test but not in the reference.)

r=dbaron with that

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

https://hg.mozilla.org/integration/mozilla-inbound/rev/af96c149900b
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7e75614e955

Comment 13

[Xidorn Quan [:xidorn] UTC+11]

Ah, I guess I should always include tests on ICS when push to try. It meets the same crash which happens on patches for bug 1083004. Make this bug blocked by bug 1052924.

Comment 14

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/28be41369dfe

Comment 15

[Xidorn Quan [:xidorn] UTC+11]

with bug 1052924:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3928aae210c3
https://hg.mozilla.org/integration/mozilla-inbound/rev/b657497010c2

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3928aae210c3
https://hg.mozilla.org/mozilla-central/rev/b657497010c2