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
Created attachment 24734 [details] Testcase
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
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]
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
Created attachment 57313 [details] Proposed Patch
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...")
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.
Created attachment 57749 [details] Proposed Patch
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 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..)
Safari, Chrome, and Firefox show the same rendering behavior for this test case. I do not believe any further compatibility issue remains.