Bug 38891 – REGRESSION (r57657): Incorrect font-size of :first-letter when using cufon.js

• Component: CSS
• Status: RESOLVED FIXED

[Philippe Wittenbergh]

Created attachment 55675 [details]
screenshot

It is enough to have the page link to the script, it does not need to be actually used.
Minimal testcase:
http://dev.l-c-n.com/webkit/test.html

The font-size in the test case is twice as large as it should be.
Safari 4.05, Gecko 1.9.2 (fx 3.6.3) and Opera 10.5 are correct.

This regressed between WebKit-SVN-r57509 and WebKit-SVN-r57720

cufon.js
http://cufon.shoqolate.com/generate/

(only tested on 10.6.3)

In the attached screenshot, on the left WebKit nightly build, on the right Safari 4.05

[Alexey Proskuryakov]

Works in r57654, fails in r57659, and r57657 is the only suspicious change in the range.

https://bugs.webkit.org/show_bug.cgi?id=37567, :first-letter inside a :visited link is wrong color.  Make sure that the pseudo style caching allows visited link styles to hang off other pseudo styles.

[Philippe Wittenbergh]

If you want some more fun :~]

1. Load http://l-c-n.com/
   (pay attention to the size of the first letter in the headlines)
2. make window narrower (less than ~650px)
3. the layout changes (correct - media query kicks in)
   but the size of the :first-letter changes ! (becomes correct)

[Alexey Proskuryakov]

<rdar://problem/7974548>

[Dave Hyatt]

Created attachment 55881 [details]
Patch

[WebKit Review Bot]

Attachment 55881 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/rendering/RenderBlock.cpp:4654:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 55881 [details]
Patch

> -    while (currChild && currChild->needsLayout() && ((!currChild->isReplaced() && !currChild->isRenderButton() && !currChild->isMenuList()) || currChild->isFloatingOrPositioned()) && !currChild->isText()) {
> +    while (currChild && ((!currChild->isReplaced() && !currChild->isRenderButton() && !currChild->isMenuList()) || currChild->isFloatingOrPositioned()) && !currChild->isText()) {

Is there a way to make this more readable with a helper function for the boolean test here?

> -            if (currChild->style()->styleType() == FIRST_LETTER)
> +            if (currChild->style()->styleType() == FIRST_LETTER) {
> +                currChild = currChild->firstChild();
>                  break;
> +            } else
> +                currChild = currChild->nextSibling();

No need for else after break.

r=me

[Dave Hyatt]

Fixed in r59247.