Bug 21937 - h1:first-letter is incorrectly inherited by run-in box
Summary: h1:first-letter is incorrectly inherited by run-in box
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 525.x (Safari 3.1)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-28 15:03 PDT by Anantha Keesara
Modified: 2022-07-11 16:33 PDT (History)
4 users (show)

See Also:


Attachments
Testcase (361 bytes, application/xhtml+xml)
2008-10-28 15:03 PDT, Anantha Keesara
no flags Details
same test case as text/html (361 bytes, text/html)
2008-12-29 04:58 PST, Robert Blaut
no flags Details
Proposed Patch (3.73 KB, patch)
2010-05-28 03:38 PDT, Yoshiki Hayashi
no flags Details | Formatted Diff | Diff
Proposed Patch (3.41 KB, patch)
2010-06-03 03:29 PDT, Yoshiki Hayashi
zimmermann: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anantha Keesara 2008-10-28 15:03:21 PDT
What steps will reproduce the problem?
1. Open the attached page.

What is the expected result?
A link named "top" 

What happens instead?
A link named "Top" - first letter in "top" should not be in uppercase

FF3: ok
Safari4(526), Chrome154.5: not ok
Comment 1 Anantha Keesara 2008-10-28 15:03:54 PDT
Created attachment 24734 [details]
Testcase
Comment 2 Robert Blaut 2008-12-29 04:58:43 PST
Created attachment 26290 [details]
same test case as text/html

I found interesting behavior when you compare rendering of test case bug sending as text/html and application/xhtml+xml
Comment 3 Robert Blaut 2008-12-29 05:05:01 PST
Confirmed as bug in both cases since "Despite appearing visually part of the following block box, a run-in element still inherits properties from its parent in the source tree." [http://www.w3.org/TR/CSS21/visuren.html#run-in]
Comment 4 Yoshiki Hayashi 2010-05-28 03:36:21 PDT
It looks like you just have to skip run-ins when you try to find the block for :first-letter.

The test in the patch I'm going to attach depends on the following bug because otherwise :first-letter applied letter doesn't show up in dumpAsText.

https://bugs.webkit.org/show_bug.cgi?id=39863
Comment 5 Yoshiki Hayashi 2010-05-28 03:38:48 PDT
Created attachment 57313 [details]
Proposed Patch
Comment 6 Shinichiro Hamaji 2010-06-02 03:45:56 PDT
Comment on attachment 57313 [details]
Proposed Patch

Looks good, but putting r- for minor issues.

I've noticed that with the following HTML

<style type="text/css">
  h1:first-letter {text-transform: uppercase;}
</style>
<h1 style="display:run-in;">capitalized</h1><div>downcase</div>

the text "capitalized" isn't capitalized even with this patch. Is this issue related to this bug? Or, should we file another bug to resolve this issue?


I think this patch depends on the patch in Bug 39863 so we should not land this patch without fixing Bug 39863. For such cases, please set commit-queue- to tell reviewers/committers that this patch should not be landed.


LayoutTests/ChangeLog:8
 +          Need a short description and bug URL (OOPS!)
This line should be gone.

WebCore/ChangeLog:8
 +          Need a short description and bug URL (OOPS!)
Unnecessary. Or, you might want to describe something.

LayoutTests/fast/css/first-letter-after-run-in.html:18
 +  <div class="run-in">run-in</div><div class="upper">test</div>
I would say

<div class="run-in">not-capitalized</div><div class="upper">capitalized</div>

or something like this so we can remove the description about the expectation ("If you see...")
Comment 7 Yoshiki Hayashi 2010-06-03 03:25:25 PDT
Thank you for review!

(In reply to comment #6)
> (From update of attachment 57313 [details])
> Looks good, but putting r- for minor issues.
> 
> I've noticed that with the following HTML
> 
> <style type="text/css">
>   h1:first-letter {text-transform: uppercase;}
> </style>
> <h1 style="display:run-in;">capitalized</h1><div>downcase</div>
> 
> the text "capitalized" isn't capitalized even with this patch. Is this issue related to this bug? Or, should we file another bug to resolve this issue?

That's a different bug.  Right now, WebKit doesn't apply first-letter to inline elements.  In this case, the h1 with run-in is converted to an inline element and inserted as the first element of the div so it won't get :first-letter.  The following link shows that div gets :first-letter while span does not.

http://www.plexode.com/cgi-bin/eval3.py#ht=%3Cstyle%3E%0Adiv%3A%3Afirst-letter%20%7B%0A%20%20text-transform%3A%20uppercase%3B%0A%7D%0Aspan%3A%3Afirst-letter%20%7B%0A%20%20text-transform%3A%20uppercase%3B%0A%7D%0A%3C%2Fstyle%3E%0A%0A%3Cdiv%3E%0Atest%0A%3C%2Fdiv%3E%0A%0A%3Cspan%3E%0Atest%0A%3C%2Fspan%3E%0A&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1

> 
> I think this patch depends on the patch in Bug 39863 so we should not land this patch without fixing Bug 39863. For such cases, please set commit-queue- to tell reviewers/committers that this patch should not be landed.
> 
> 
> LayoutTests/ChangeLog:8
>  +          Need a short description and bug URL (OOPS!)
> This line should be gone.
> 
> WebCore/ChangeLog:8
>  +          Need a short description and bug URL (OOPS!)
> Unnecessary. Or, you might want to describe something.

Oops.  Removed.

> LayoutTests/fast/css/first-letter-after-run-in.html:18
>  +  <div class="run-in">run-in</div><div class="upper">test</div>
> I would say
> 
> <div class="run-in">not-capitalized</div><div class="upper">capitalized</div>
> 
> or something like this so we can remove the description about the expectation ("If you see...")

Done.

I also added a test case where run-in stays as a block level element and if :first-letter gets applied to that element.
Comment 8 Yoshiki Hayashi 2010-06-03 03:29:10 PDT
Created attachment 57749 [details]
Proposed Patch
Comment 9 Shinichiro Hamaji 2010-06-03 03:44:45 PDT
Comment on attachment 57749 [details]
Proposed Patch

Looks good. Thanks for your description and updating the patch!

> That's a different bug.  Right now, WebKit doesn't apply first-letter to inline elements.  In this case, the h1 with run-in is converted to an inline element and inserted as the first element of the div so it won't get :first-letter.  The following link shows that div gets :first-letter while span does not.

Ah, I didn't notice run-in box becomes an inline element. It seems the current spec says we don't need to apply ::first-letter for inline elements, so this would be a non-issue.

http://dev.w3.org/csswg/selectors3/#application-in-css
Comment 10 Nikolas Zimmermann 2010-07-30 23:23:18 PDT
Comment on attachment 57749 [details]
Proposed Patch

LayoutTests/fast/css/first-letter-after-run-in-expected.txt:1
 +  not-capitalized
Can you use the script-tests framework, to create a better testcase, that actually tells what you're checkink. Of course it's clear from the testcase, but I think you could do better and explicitely explain what's happening, in the expected-txt output.

Can't review the actual RenderBlock change, you'll need to ask rendering gurus for that (Dave Hyatt / Dan Bernstein..)
Comment 11 Brent Fulgham 2022-07-11 16:33:51 PDT
Safari, Chrome, and Firefox show the same rendering behavior for this test case. I do not believe any further compatibility issue remains.