| Summary: | CSSFontStyleValue::isItalic seems a bit bogus. | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Emilio Cobos Álvarez <ecobos> | ||||
| Component: | CSS | Assignee: | Myles C. Maxfield <mmaxfield> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | commit-queue, dino, hyatt, jonlee, koivisto, mmaxfield, sandervv+webkit, simon.fraser, thorton, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
Good catch! Created attachment 314662 [details]
Patch
Comment on attachment 314662 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314662&action=review > Source/WebCore/ChangeLog:12 > + * css/CSSFontStyleValue.h: Write some words! > Source/WebCore/css/CSSFontStyleValue.h:50 > { Should this be called isItalicOrOblique? > LayoutTests/editing/execCommand/italicizeByCharacter-normal.html:11 > +<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script> This is the ugliest <script> I've seen in a long time :) Comment on attachment 314662 [details] Patch Clearing flags on attachment: 314662 Committed r219173: <http://trac.webkit.org/changeset/219173> All reviewed patches have been landed. Closing bug. (In reply to Tim Horton from comment #3) > Comment on attachment 314662 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=314662&action=review > > > Source/WebCore/ChangeLog:12 > > + * css/CSSFontStyleValue.h: > > Write some words! > > > Source/WebCore/css/CSSFontStyleValue.h:50 > > { > > Should this be called isItalicOrOblique? > > > LayoutTests/editing/execCommand/italicizeByCharacter-normal.html:11 > > +<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script> > > This is the ugliest <script> I've seen in a long time :) Oh no, I didn't actually do anything you said!!! :( I'll do it in a follow-up patch. Committed r220531: <http://trac.webkit.org/changeset/220531> Comment on attachment 314662 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314662&action=review > Source/WebCore/ChangeLog:3 > + CSSFontStyleValue::isItalic seems a bit bogus. Vaguest bug title ever. > > + CSSFontStyleValue::isItalic seems a bit bogus.
>
> Vaguest bug title ever.
"CSSFontStyleValue::isItalic has high bogosity."
*** Bug 173970 has been marked as a duplicate of this bug. *** |
bool isItalic() const { if (!obliqueValue) { auto valueID = fontStyleValue->valueID(); return valueID == CSSValueItalic || CSSValueOblique; } return obliqueValue->value<float>(CSSPrimitiveValue::CSS_DEG) >= static_cast<float>(italicValue()); } Note how "|| CSSValueOblique" makes the first return always return true. I'm not familiar with this code, so I'm not sure how to best test it. Happy to fix it with indications on how to test it. I've CCd the original patch author and reviewer so you can take a look (and fix) if you want to.