[Webkit-unassigned] [Bug 52952] Focus ring for anchor with inline image is incorrect

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52952





--- Comment #40 from Antonio Gomes <tonikitoo at webkit.org>  2011-07-02 16:31:19 PST ---
(From update of attachment 89913)
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_"

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.