Closed Bug 1084183 Opened 5 years ago Closed 5 years ago

Propagate text decoration to ruby frames

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(2 files, 2 obsolete files)

Currently, text decorations are not propagated into ruby frames.
Attached patch patchSplinter Review
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.
Assignee: nobody → quanxunzhen
Status: NEW → ASSIGNED
Attachment #8507607 - Flags: review?(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.
Attachment #8507607 - Flags: review?(dbaron) → review+
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.
Keywords: checkin-needed
I forgot that I didn't write test for it. Remove checkin-needed.
Keywords: checkin-needed
Attached patch patch of test (obsolete) — Splinter Review
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.
Attachment #8512433 - Flags: review?(dbaron)
Depends on: 1090744
Depends on: 1068477
Attached patch patch of test (obsolete) — Splinter Review
Attachment #8512433 - Attachment is obsolete: true
Attachment #8512433 - Flags: review?(dbaron)
Attachment #8513896 - Flags: review?(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.
Attachment #8513896 - Flags: review?(dbaron) → review+
My patch 1 of bug 1088489 causes some problems in the test for this bug. The problems will be fixed by the patch 2.
Depends on: 1088489
No longer depends on: 1088489
Attached patch patch of testSplinter Review
Changes:
1. The directory name is changed to text-decor-3
2. Wrap content in <rb> with <span>
3. Remove inter-segment whitespaces
Attachment #8513896 - Attachment is obsolete: true
Attachment #8523482 - Flags: review?(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
Attachment #8523482 - Flags: review?(dbaron) → review+
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.
Depends on: 1052924
https://hg.mozilla.org/mozilla-central/rev/3928aae210c3
https://hg.mozilla.org/mozilla-central/rev/b657497010c2
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.