[Webkit-unassigned] [Bug 133796] ASSERT_NOT_REACHED() in WebCore::fontWeightIsBold

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=133796





--- Comment #22 from Ryosuke Niwa <rniwa at webkit.org>  2014-07-16 12:18:02 PST ---
(From update of attachment 234923)
View in context: https://bugs.webkit.org/attachment.cgi?id=234923&action=review

>> LayoutTests/ChangeLog:12
>> +        * css1/font_properties/font_weight_lighter.html: Added.
> 
> Test cases should be reference tests.
> 
> I think it’s really strange to have these tests in the css1 directory. These are editing tests and belong in the editing directory.
> 
> In a CSS test directory (not this css1 one) we could have tests to make sure that bolder and lighter are rejected as keyword values during the CSS parsing process. Only a few special paths in editing code should parse in a mode that allows these values.

bolder and lighter are real CSS2.1 values:
http://www.w3.org/TR/CSS21/fonts.html#font-boldness

>> Source/WebCore/editing/EditingStyle.cpp:1572
>> +    }
> 
> If fontWeight is bolder or lighter, then we should skip removeProperty regardless of the value of baseFontWeight. That needs to be done here; I don’t see a way to correctly encapsulate it in the fontWeightIsBold helper function.
> 
> If the font weight is the minimum the we could remove “lighter”, and if the font weight is the maximum we could remove “bolder”. That would be nice to get right, but it’s an edge case that I don’t think is critical.
> 
> I’d like to see test cases covering this behavior, including the edge cases.
> 
> If baseFontWeight is bolder or lighter, then the bug has already happened. Such styles should never be allowed in CSS styles on actual elements. Those values are a hack to be used inside the editing machinery and in API not exposed to JavaScript. If JavaScript can ever see those values, then we have a bug.
> 
> The refactoring to use references instead of pointers we are doing here messes up the logic here and makes this function harder to read. We should think about how to handle that, and why boldness is different from the other properties below in this respect. I suspect that refactoring is not really part of this bug fix and we should consider doing it separately.

lighter and bolder are valid CSS values.  They bolden & lighten text weight relative to the inherited font weight.
I do agree that one correct approach is avoid removing font-weight property when those values are used.

Alternatively, we can also try to resolve the computed font weight by pluming values from render style but that's going to be a lot of work.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.