Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix text-decoration property in layout & fix line-height property #1506

Merged
merged 5 commits into from
Jan 23, 2014

Conversation

ksh8281
Copy link
Contributor

@ksh8281 ksh8281 commented Jan 16, 2014

add inline stuff & color support
it makes same result with firefox in "http://www.w3.org/Style/CSS/Test/CSS1/current/sec543.htm"
makes line-height property can inherit

@highfive
Copy link

warning Warning warning

  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!
  • @ksh8281, please confirm that src/test/html/acid1.html and your favourite wikipedia page still render correctly!

@ksh8281
Copy link
Contributor Author

ksh8281 commented Jan 20, 2014

r? @pcwalton

@pcwalton
Copy link
Contributor

This makes sense, but it seems rare that underline/overline/text-decoration could be a different color from the color. Maybe we should not store separate colors in every flow, but rather have something like Option<~RareFlowFlags> with:

struct RareFlowFlags {
    underline_color: Color,
    overline_color: Color,
    line_through_color: Color,
}

If rare_flow_flags is None, then it would be assumed that the underline/overline/line through colors are the same as the text color. This would save memory in the common case.

@ksh8281
Copy link
Contributor Author

ksh8281 commented Jan 22, 2014

@pcwalton i think there is no way to compare color when propagate data.
because there is not information about color or style.
so i apply your opinion in other way.
r?

bors-servo pushed a commit that referenced this pull request Jan 23, 2014
add inline stuff & color support
it makes same result with firefox in "http://www.w3.org/Style/CSS/Test/CSS1/current/sec543.htm"
makes line-height property can inherit
@bors-servo bors-servo merged commit 4133982 into servo:master Jan 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants