Bug 52952 - Focus ring for anchor with inline image is incorrect
: Focus ring for anchor with inline image is incorrect
Status: NEW
: WebKit
New Bugs
: 528+ (Nightly build)
: PC All
: P2 Normal
Assigned To:
:
:
:
: 46905
  Show dependency treegraph
 
Reported: 2011-01-22 00:49 PST by
Modified: 2012-04-19 15:19 PST (History)


Attachments
It's a tc. Just try to open index.html and then press tab key. (49.60 KB, application/x-gzip)
2011-01-22 00:49 PST, ChangSeok Oh
no flags Details
More simple TC (5.77 KB, application/x-gzip)
2011-02-21 07:32 PST, ChangSeok Oh
no flags Details
The image is replaced. (407 bytes, text/html)
2011-03-01 18:06 PST, ChangSeok Oh
no flags Details
a proposed patch (1.28 KB, patch)
2011-03-10 23:23 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
a proposed patch (1.25 KB, patch)
2011-03-10 23:51 PST, ChangSeok Oh
tonikitoo: review-
tonikitoo: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
proposed patch (44.75 KB, patch)
2011-03-15 09:42 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
Updated ChangeLogs and resolved crash issue that Yael mentioned (45.44 KB, patch)
2011-03-16 12:06 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
updated patch (45.31 KB, patch)
2011-03-17 09:45 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
updated patch (45.36 KB, patch)
2011-03-21 09:28 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
updated patch (45.52 KB, patch)
2011-03-24 01:15 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
updated patch (74.02 KB, patch)
2011-03-27 08:55 PST, ChangSeok Oh
eric: review-
Review Patch | Details | Formatted Diff | Diff
updated patch (38.23 KB, patch)
2011-04-12 10:45 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch to meet Eric's comment. (38.90 KB, patch)
2011-04-15 21:36 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
updated patch (37.97 KB, patch)
2011-07-05 07:54 PST, ChangSeok Oh
webkit.review.bot: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.25 MB, application/zip)
2011-07-05 08:20 PST, WebKit Review Bot
no flags Details
updated patch (40.06 KB, patch)
2011-07-06 04:01 PST, ChangSeok Oh
no flags Review Patch | Details | Formatted Diff | Diff
Patch (27.97 KB, patch)
2012-01-06 08:37 PST, ChangSeok Oh
eric: review-
webkit.review.bot: commit‑queue-
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-01-22 00:49:02 PST
Created an attachment (id=79835) [details]
It's a tc. Just try to open index.html and then press tab key.

Hello. folks.
I'm using Ubuntu 10.10 32bits system but this issue is shown in all platform.

When I navigated YouTube using by tab. I found this issue.
The focused area is smaller than I expect.

Find attached TC.
Open it and just press tab-key.

There is a image which is a child node of anchor node.
I think that the focused area should include image region but it doesn't.
The focused region is for only anchor node.

I spent some time to track this issue.
It seems WebCore make right invalidate region. 
But Actually rendered region is for anchor region except image region.
------- Comment #1 From 2011-02-21 07:32:26 PST -------
Created an attachment (id=83164) [details]
More simple TC
------- Comment #2 From 2011-02-26 08:54:40 PST -------
I can reproduce this bug on the trunk.

Changseok, could you upload the same test case as html file?
------- Comment #3 From 2011-03-01 18:06:47 PST -------
Created an attachment (id=84340) [details]
The image is replaced.
------- Comment #4 From 2011-03-01 18:26:02 PST -------
I found the reason of this issue and created a patch to solve.
But I haven't been able to run regression test(run-webkit-test) on my Ubuntu system(10.10).
Is it possible to run this on Ubuntu system?
I'm faceing following log message when I run regression test(run-webkit-test).
>WARNING: Your platform is not recognized. Any platform-specific results will be generated in >platform/undefined.
>Running build-dumprendertree
>Building not defined for this platform!
>Compiling DumpRenderTree failed!

What does this mean?
------- Comment #5 From 2011-03-03 08:53:54 PST -------
> There is a image which is a child node of anchor node.
> I think that the focused area should include image region but it doesn't.
> The focused region is for only anchor node.
> 
> I spent some time to track this issue.
> It seems WebCore make right invalidate region. 
> But Actually rendered region is for anchor region except image region.

I've seen this, and I confirm it is a bug. I will have a look.
------- Comment #6 From 2011-03-10 23:23:18 PST -------
Created an attachment (id=85438) [details]
a proposed patch

I have done regression tests. It seems no problem.
------- Comment #7 From 2011-03-10 23:51:01 PST -------
Created an attachment (id=85439) [details]
a proposed patch

There is mimic change removing type casting.
------- Comment #8 From 2011-03-11 04:50:02 PST -------
(From update of attachment 85439 [details])
It needs a very least one layout test (and possibly one pixel test)
------- Comment #9 From 2011-03-11 09:29:25 PST -------
(In reply to comment #8)
> (From update of attachment 85439 [details] [details])
> It needs a very least one layout test (and possibly one pixel test)

Thanks for your comment. :)

By the way, I couldn't get your point. 
Do you want regression test result or new test cases to evaluate this patch?
------- Comment #10 From 2011-03-11 10:04:21 PST -------
(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 85439 [details] [details] [details])
> > It needs a very least one layout test (and possibly one pixel test)
> 
> Thanks for your comment. :)
> 
> By the way, I couldn't get your point. 
> Do you want regression test result or new test cases to evaluate this patch?

ChangSeok, please read these: http://www.webkit.org/blog/1452/layout-tests-theory/ and http://www.webkit.org/blog/1456/layout-tests-practice/
------- Comment #11 From 2011-03-14 01:25:03 PST -------
ChangSeok, you have to add a changeLog to your patch.

http://www.webkit.org/coding/contributing.html#changelogs
------- Comment #12 From 2011-03-15 09:42:15 PST -------
Created an attachment (id=85818) [details]
proposed patch

Patch updated ChangeLog & test case.

Thanks, Antonio & Joone :)

I think that layout test is optional but pixel test is mandatory.
Because this patch doesn't affect DOM and layout tree.

I'm not sure whether the test case is placed at proper directory.
Please let me know, if it's false.
------- Comment #13 From 2011-03-15 09:56:13 PST -------
(From update of attachment 85818 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=85818&action=review

You are only providing mac results. Maybe you plan to watch the bots to grab expected results for other ports (gtk, qt, chromium, win)

> Source/WebCore/ChangeLog:6
> +        Wrong tab focused region
> +        https://bugs.webkit.org/show_bug.cgi?id=52952

here you need to say what was the bug and how you fixed in more details.

> LayoutTests/ChangeLog:7
> +        Wrong tab focused region
> +        https://bugs.webkit.org/show_bug.cgi?id=52952
> +

Ditto
------- Comment #14 From 2011-03-16 07:05:30 PST -------
(From update of attachment 85818 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=85818&action=review

> Source/WebCore/rendering/RenderInline.cpp:1039
> +                top = min(top, toRenderBox(currRenderObj)->logicalTop());

I added a span around the image of the attached sample page, and tested with this patch. It crashed because currRenderObj was not a RenderBox.
------- Comment #15 From 2011-03-16 12:06:43 PST -------
Created an attachment (id=85952) [details]
Updated ChangeLogs and resolved crash issue that Yael mentioned

> You are only providing mac results. Maybe you plan to watch the bots to grab expected results for other ports (gtk, qt, chromium, win)
Should I also submit other ports results? Hm... 
I haven't used qt so it sounds a little hard. :p

>> Source/WebCore/ChangeLog:6
>> +        Wrong tab focused region
>> +        https://bugs.webkit.org/show_bug.cgi?id=52952
>here you need to say what was the bug and how you fixed in more details.
Done. I added more description about this patch.

> LayoutTests/ChangeLog:7
> +        Wrong tab focused region
> +        https://bugs.webkit.org/show_bug.cgi?id=52952
> +
Done.

In addition. The crash issue that Yael mentioned is resolved.
------- Comment #16 From 2011-03-16 13:30:14 PST -------
(In reply to comment #15)
> Created an attachment (id=85952) [details] [details]
> Updated ChangeLogs and resolved crash issue that Yael mentioned
> 
> > You are only providing mac results. Maybe you plan to watch the bots to grab expected results for other ports (gtk, qt, chromium, win)
> Should I also submit other ports results? Hm... 
> I haven't used qt so it sounds a little hard. :p

Unless you want to break all other ports, yes. You can watch the bots and grab results OR you can skip the tests for other ports (you do not want that).
------- Comment #17 From 2011-03-16 14:46:45 PST -------
(From update of attachment 85952 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=85952&action=review

> Source/WebCore/rendering/RenderInline.cpp:1038
>      for (InlineFlowBox* curr = firstLineBox(); curr; curr = curr->nextLineBox()) {
>          RootInlineBox* root = curr->root();
>          int top = max(root->lineTop(), curr->logicalTop());
>          int bottom = min(root->lineBottom(), curr->logicalBottom());
> +    
> +        Vector<IntRect> focusRingRects;
> +        addFocusRingRects(focusRingRects, 0, 0);

addFocusRingRects should not be inside the for loop. 
Do we need to have both the original for loop and addFocusRingRects ?
------- Comment #18 From 2011-03-17 09:45:54 PST -------
Created an attachment (id=86063) [details]
updated patch

> addFocusRingRects should not be inside the for loop. 
> Do we need to have both the original for loop and addFocusRingRects ?
We don't. I agree that using addFocusRingRects was wrong approach.
Here is a updated patch.
------- Comment #19 From 2011-03-20 08:18:16 PST -------
I don't think this is right. Please see https://bugs.webkit.org/show_bug.cgi?id=28625#c1 .
------- Comment #20 From 2011-03-20 09:20:33 PST -------
(In reply to comment #19)
> I don't think this is right. Please see https://bugs.webkit.org/show_bug.cgi?id=28625#c1 .

Yael, could you please advice him the right way?
------- Comment #21 From 2011-03-20 10:12:42 PST -------
(In reply to comment #19)
> I don't think this is right. Please see https://bugs.webkit.org/show_bug.cgi?id=28625#c1 .

Would you explain why?
I had a view bug28625, but I couldn't still get your point. ''a
When I run the TC of 28625 with patch and without patch, I get same results.
Actually this patch may not affect b28625. 
The focus ring drawn in 28625 TC is different from this bug's one.
It doesn't use css outline to draw focus ring, even though it doesn't reach at RenderInline::paintOutline to render it when I move focus with tab key.

With this patch. I could get proper outline for anchored image like IE9, Opera10 and FF4.
Please let me know your opinion in detail.
Thanks :)
------- Comment #22 From 2011-03-20 14:48:21 PST -------
(In reply to comment #21)
> (In reply to comment #19)
> > I don't think this is right. Please see https://bugs.webkit.org/show_bug.cgi?id=28625#c1 .
> 
> Would you explain why?
> I had a view bug28625, but I couldn't still get your point. ''a
> When I run the TC of 28625 with patch and without patch, I get same results.
> Actually this patch may not affect b28625. 
> The focus ring drawn in 28625 TC is different from this bug's one.
> It doesn't use css outline to draw focus ring, even though it doesn't reach at RenderInline::paintOutline to render it when I move focus with tab key.
> 
> With this patch. I could get proper outline for anchored image like IE9, Opera10 and FF4.
> Please let me know your opinion in detail.
> Thanks :)

I was afraid that you are reverting the fix from 28625 :) I have no objection to your patch if you tested the other test case is still passing.
------- Comment #23 From 2011-03-21 03:18:07 PST -------
(In reply to comment #22)
> I was afraid that you are reverting the fix from 28625 :) I have no objection to your patch if you tested the other test case is still passing.

Yael, I ran layout test(including pixel test) again on Ubuntu 10.10 & SnowLeopard, but I couldn't find any issues that this patch make.
Of course, 28625 TC too.

Please let me know another issue you're concerned(if exist)
------- Comment #24 From 2011-03-21 09:28:44 PST -------
Created an attachment (id=86324) [details]
updated patch

Fix a bug found in real site.
http://www.youtube.com/comment_servlet?all_comments=1&v=2uS8cd2OXjE (Need to log in)
'Cancel' focused region was wrong.
------- Comment #25 From 2011-03-24 01:15:18 PST -------
Created an attachment (id=86753) [details]
updated patch

I fixed more specific case bug in real site.
------- Comment #26 From 2011-03-24 07:03:21 PST -------
(In reply to comment #25)
> Created an attachment (id=86753) [details] [details]
> updated patch
> 
> I fixed more specific case bug in real site.

Are you updating your test cases so these "real site" problems wont regress? it is also important ...
------- Comment #27 From 2011-03-27 08:55:12 PST -------
Created an attachment (id=87072) [details]
updated patch

The test case is updated. It expands to verify real site issue found.
------- Comment #28 From 2011-04-04 04:00:08 PST -------
Any progress? Somebody review this, please.
------- Comment #29 From 2011-04-04 04:08:18 PST -------
This looks about right to me.  Best if hyatt were to take a look, but I could also review it when my brain isn't busy with bidi text.  Ping me in a week if you haven't heard from hyatt.
------- Comment #30 From 2011-04-04 22:33:28 PST -------
(In reply to comment #29)
> This looks about right to me.  Best if hyatt were to take a look, but I could also review it when my brain isn't busy with bidi text.  Ping me in a week if you haven't heard from hyatt.

O.K Thanks you! Eric :)
------- Comment #31 From 2011-04-11 06:10:09 PST -------
(From update of attachment 87072 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=87072&action=review

I think we need another round and some questions answered.  I'm sorry I didn't look at this sooner.  Thank you very much for the reminder email!

> Source/WebCore/ChangeLog:5
> +        Wrong tab focused region

You might want to update the bug titles in the ChangeLogs.  Not important, but the new title is definitely more descriptive than the old one. :)

> Source/WebCore/rendering/RenderInline.cpp:1039
> +        for (InlineBox* child = curr->firstChild(); child; child = child->next()) {

Hmm... Maybe we should be caching this information on the root line boxes instead?

How is this affected by hyatt's recent change of removing many lineboxes from the linebox tree?

> Source/WebCore/rendering/RenderInline.cpp:1041
> +            if (!renderObj->isText()) {

I take it the text case is already handled somewhere else?

It seems we should be saving this information on the parent linebox as part of layout()?  But I iadmit not to be a painting expert for the linebox tree yet.

> LayoutTests/fast/repaint/outline-focus-anchor-image.html:4
> +    <title>CSS 2.1 Test Suite: outline</title>

So is this a modified version of a CSS2.1 test?  Or is this just imported directly from CSS 2.1's test suite? I believe we already have the css 2.1 test suite imported into LayoutTests.  Have you run all the layout tests to see if your change affects any of the rest of them?

> LayoutTests/fast/repaint/outline-focus-anchor-image.html:25
> +        Text before image anchor <img src="resources/apple.jpg" width=100px height=100px /><br>

apple.jpg seems to be missing from your patch.  Again, you might not even need this test as it may already be in our repository, unless you modified an existing CSS21 test?
------- Comment #32 From 2011-04-12 10:43:31 PST -------
(From update of attachment 87072 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=87072&action=review

>> Source/WebCore/ChangeLog:5
>> +        Wrong tab focused region
> 
> You might want to update the bug titles in the ChangeLogs.  Not important, but the new title is definitely more descriptive than the old one. :)

First of all. Thank you for review! :)
Sure. I update ChangLog.

>> Source/WebCore/rendering/RenderInline.cpp:1039
>> +        for (InlineBox* child = curr->firstChild(); child; child = child->next()) {
> 
> Hmm... Maybe we should be caching this information on the root line boxes instead?
> 
> How is this affected by hyatt's recent change of removing many lineboxes from the linebox tree?

A new variable is created in InlineFlowBox class to hold condition that InlineFlowBox has text children only.
And new InlineBox is checked whether it's text or not when it's added to InlineFlowBox.
In this way, we can avoid a loop like this line.
Hyatt's recent change? Do you mean bug28625 Yael mentioned above? When I test bug28625, this patch doesn't affect it.

>> Source/WebCore/rendering/RenderInline.cpp:1041
>> +            if (!renderObj->isText()) {
> 
> I take it the text case is already handled somewhere else?
> 
> It seems we should be saving this information on the parent linebox as part of layout()?  But I iadmit not to be a painting expert for the linebox tree yet.

Yeh. similar checking have been doing in InlineFlowBox::addToLine. A new variable in InlineFlowBox will hold the condition.

>> LayoutTests/fast/repaint/outline-focus-anchor-image.html:4
>> +    <title>CSS 2.1 Test Suite: outline</title>
> 
> So is this a modified version of a CSS2.1 test?  Or is this just imported directly from CSS 2.1's test suite? I believe we already have the css 2.1 test suite imported into LayoutTests.  Have you run all the layout tests to see if your change affects any of the rest of them?

No. This is NOT formal CSS 2.1 test. This test case is written by myself. I just use CSS category of outline as title. :)
The title is changed to avoid confusion.
Of course, I've run layout tests(including pixel tests) several times for this patch.  No problem as I know.

>> LayoutTests/fast/repaint/outline-focus-anchor-image.html:25
>> +        Text before image anchor <img src="resources/apple.jpg" width=100px height=100px /><br>
> 
> apple.jpg seems to be missing from your patch.  Again, you might not even need this test as it may already be in our repository, unless you modified an existing CSS21 test?

apple.jpg was only one available image in LayoutTest/fast/repaint directory when I wrote this TC.
I didn't know that adding new image for TC was O.K. I'll replace apple.jpg with greenSquare.png submitted newly.
------- Comment #33 From 2011-04-12 10:45:47 PST -------
Created an attachment (id=89225) [details]
updated patch

updated patch to meet Eric's comment.
------- Comment #34 From 2011-04-15 09:53:55 PST -------
(From update of attachment 89225 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=89225&action=review

This looks reasonable to me, but I think we should rope hyatt or mitz in here still.  Lets see if we can find one of them in #webkit today.

> Source/WebCore/rendering/InlineFlowBox.cpp:109
> +    } else {
> +        m_hasTextChildrenOnly = false;
>      }

Style nit:  no { } on single line blocks.  I'm surprised the style queue didn't flag this.

> Source/WebCore/rendering/InlineFlowBox.h:293
> +    bool m_hasTextChildrenOnly : 1;

I probably would have named this m_hasOnlyTextChildren.

So this is only children?  Or is this true of all dependents?  Do we keep this up to date when grafting on sub-trees?  I'm not sure what linebox tree manipulations are allowed that we'd need to worry about.
------- Comment #35 From 2011-04-15 09:56:39 PST -------
I'm sorry this patch has been so much trouble.  The lineboxtree is a non-trivial piece of the webkit code which few people are familiar with.
------- Comment #36 From 2011-04-15 21:33:01 PST -------
(From update of attachment 89225 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=89225&action=review

Thanks for review again. :)

>> Source/WebCore/rendering/InlineFlowBox.cpp:109
>>      }
> 
> Style nit:  no { } on single line blocks.  I'm surprised the style queue didn't flag this.

My mistake. Done.

>> Source/WebCore/rendering/InlineFlowBox.h:293
>> +    bool m_hasTextChildrenOnly : 1;
> 
> I probably would have named this m_hasOnlyTextChildren.
> 
> So this is only children?  Or is this true of all dependents?  Do we keep this up to date when grafting on sub-trees?  I'm not sure what linebox tree manipulations are allowed that we'd need to worry about.

Done.

m_hasTextChildrenOnly just hold a condition whether InlineFlowBox has only text type InlineBox child.
In my understanding, current paintOutline implementation for css:outline has taken care of only text string. Because of InlineFlowBox's logicalTop.
When the top of outline is decided, the logical top of InlineFlowBox is refered.
Although RootInlineBox has proper top position, it could not be bigger than top of text line.

I tested another case for checking m_hasOnlyTextChildren is kept up to date.
Yes, it is. 
I added a new image child to parentAnchor node using by setTimeout JS function.
The outline was expanded successfully like FF and Opera. Why it does is that m_hasOnlyTextChildren is kept up to date in InlineFlowBox::addToLine when a new InlineBox is added to InlineFlowBox.
------- Comment #37 From 2011-04-15 21:36:21 PST -------
Created an attachment (id=89913) [details]
Updated patch to meet Eric's comment.
------- Comment #38 From 2011-04-17 22:33:16 PST -------
(In reply to comment #34)
> (From update of attachment 89225 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89225&action=review
> 
> This looks reasonable to me, but I think we should rope hyatt or mitz in here still.  Lets see if we can find one of them in #webkit today.

Eric. Would you let me know hyatt and mitz's irc IDs?
------- Comment #39 From 2011-04-18 07:53:57 PST -------
dhyatt and mitzpettel.  IRC names are listed for most committers in Tools/Scripts/webkitpy/common/config/committers.py or on wiki.webkit.org
------- Comment #40 From 2011-07-02 16:31:19 PST -------
(From update of attachment 89913 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=89913&action=review

Please Hyatt or Mitz, would you mind to have a look?

Author, you are adding a non-port-specific test, but adding Mac-specific results only. Just be sure to follow up grabbing results from the bots to update the other platforms (qt, gtk, windows and their webkit2 variants, etc).

> Source/WebCore/ChangeLog:12
> +        So font height is only calculated to get top position.
> +        To fix this, Some lines are added to get maxium line height and to recalulate top position. 

Typo:
- "Some" does not need capital "S".
- "maxium"

> Source/WebCore/rendering/InlineFlowBox.cpp:106
>              m_hasTextDescendants = true;
> -    }
> +        m_hasOnlyTextChildren = false;

"Descendants" x "Children" is a bit confusing here, but ok. It is not your patch's fault.

> LayoutTests/ChangeLog:9
> +        Test that the outline of anchor node which has non-text child node like image node.
> +        Layout test wouldn't verify this. Pixel test is valid. 

A spatial navigation test could easily very this, btw.

Pixel test is also ok, but as a follow up, it would be great to add one too.

> LayoutTests/fast/repaint/outline-focus-anchor-image.html:18
> +      <a id="ID_parentAnchor" href="#">

Nit: not need for "ID_"
------- Comment #41 From 2011-07-03 17:30:11 PST -------
(In reply to comment #40)
> Author, you are adding a non-port-specific test, but adding Mac-specific results only. Just be sure to follow up grabbing results from the bots to update the other platforms (qt, gtk, windows and their webkit2 variants, etc).
>> Thank you for comments! :)
>> May I ask you an ignorant question?
>> As your comment, I could get other port's test results without building them. Right? You said I could get the results by just "watching bot and grabbing results". Hm.. I can't understand this. How could I do? 
Please let me know where I could see the bot's activity, so grab results of other ports.
------- Comment #42 From 2011-07-03 17:34:18 PST -------
(In reply to comment #41)
> (In reply to comment #40)
> > Author, you are adding a non-port-specific test, but adding Mac-specific results only. Just be sure to follow up grabbing results from the bots to update the other platforms (qt, gtk, windows and their webkit2 variants, etc).
> >> Thank you for comments! :)
> >> May I ask you an ignorant question?
> >> As your comment, I could get other port's test results without building them. Right? You said I could get the results by just "watching bot and grabbing results". Hm.. I can't understand this. How could I do? 
> Please let me know where I could see the bot's activity, so grab results of other ports.

You can access the result of all core bots, for each revision tested from here: http://build.webkit.org/results/
------- Comment #43 From 2011-07-03 18:21:59 PST -------
(In reply to comment #42)
> (In reply to comment #41)
> > (In reply to comment #40)
> > > Author, you are adding a non-port-specific test, but adding Mac-specific results only. Just be sure to follow up grabbing results from the bots to update the other platforms (qt, gtk, windows and their webkit2 variants, etc).
> > >> Thank you for comments! :)
> > >> May I ask you an ignorant question?
> > >> As your comment, I could get other port's test results without building them. Right? You said I could get the results by just "watching bot and grabbing results". Hm.. I can't understand this. How could I do? 
> > Please let me know where I could see the bot's activity, so grab results of other ports.
> 
> You can access the result of all core bots, for each revision tested from here: http://build.webkit.org/results/
I appreciate your kind info. :)
As my understanding, the test results were made when a patch was landed on main stream after passing review. 
Then, is it O.K that I update this patch to add results for other ports after passing review(if it's possible)? If so, I'm willing to do :)
------- Comment #44 From 2011-07-05 07:54:48 PST -------
Created an attachment (id=99716) [details]
updated patch
------- Comment #45 From 2011-07-05 08:00:49 PST -------
(From update of attachment 89913 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=89913&action=review

Thank you for review.

>> Source/WebCore/ChangeLog:12
>> +        To fix this, Some lines are added to get maxium line height and to recalulate top position. 
> 
> Typo:
> - "Some" does not need capital "S".
> - "maxium"

Ops. Sorry :p

>> LayoutTests/fast/repaint/outline-focus-anchor-image.html:18
>> +      <a id="ID_parentAnchor" href="#">
> 
> Nit: not need for "ID_"

Done.
------- Comment #46 From 2011-07-05 08:20:21 PST -------
(From update of attachment 99716 [details])
Attachment 99716 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8988377

New failing tests:
fast/repaint/outline-focus-anchor-image.html
------- Comment #47 From 2011-07-05 08:20:28 PST -------
Created an attachment (id=99718) [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
------- Comment #48 From 2011-07-06 04:01:02 PST -------
Created an attachment (id=99812) [details]
updated patch
------- Comment #49 From 2011-10-13 14:24:07 PST -------
Is the problem still reproducible? Is the patch still valid? It seems there's no activity on the bug for a while.
------- Comment #50 From 2011-10-13 19:04:22 PST -------
(In reply to comment #49)
> Is the problem still reproducible? Is the patch still valid? It seems there's no activity on the bug for a while.

1) Yes

2) I am not sure if the patch is still valid. I will review it tomorrow properly.
------- Comment #51 From 2011-10-14 05:31:54 PST -------
(In reply to comment #50)
> (In reply to comment #49)
> > Is the problem still reproducible? Is the patch still valid? It seems there's no activity on the bug for a while.
> 
> 1) Yes
> 
> 2) I am not sure if the patch is still valid. I will review it tomorrow properly.

Thanks for your interest.
I've been just waiting somebody review this. It's a little hard to take a review for layout issues. :p
------- Comment #52 From 2012-01-06 08:37:32 PST -------
Created an attachment (id=121435) [details]
Patch
------- Comment #53 From 2012-01-06 09:22:44 PST -------
(From update of attachment 121435 [details])
Attachment 121435 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11141435

New failing tests:
fast/repaint/outline-focus-anchor-image.html
------- Comment #54 From 2012-04-19 15:19:04 PST -------
(From update of attachment 121435 [details])
This patch doesn't seem right.  Why do we need to create a new special case for this?