[Webkit-unassigned] [Bug 106792] Text Autosizing: don't autosize headers with multiple inline links.

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from John Mellor <johnme at chromium.org>  2013-01-16 08:56:05 PST ---
(From update of attachment 182987)
View in context: https://bugs.webkit.org/attachment.cgi?id=182987&action=review

Getting close! Final nits.

> Source/WebCore/ChangeLog:4
> +        This patch includes code for detecting rows of links typically seen

Nit: please put this description after the Reviewed by line, instead of between the bug title and bug url.

> Source/WebCore/rendering/TextAutosizer.cpp:311
> +    //  2. it should contain at least 2 inline <a> elements

s/<a> elements/links/ for consistency with the fact that we're now explicitly checking for links instead of <a> elements (see below).

> Source/WebCore/rendering/TextAutosizer.cpp:325
> +        if (node && node->isElementNode() && (toElement(node)->tagQName() == aTag)) {

Now I think about it, technically just looking for <a> elements isn't quite ideal, as that'll include anchors (i.e. <a> elements with a name attribute but no href attribute, used purely as targets of a #fragment url). Instead you can use renderer->style()->isLink() which is also shorter :)

> Source/WebCore/rendering/TextAutosizer.cpp:327
> +            // Skip traversing descendants of the <a> element

s/<a> element/link./ for consistency with the fact that we're now explicitly checking for links instead of <a> elements.

> LayoutTests/fast/text-autosizing/header-links-autosizing-expected.html:35
> +    whereas the inline links in the header above should be rendered at their default size.

Would be nice to explain why, e.g. s/should be/should be detected as a row of links and hence/ (similarly in other test). It might also be clearer to move this explanation to the paragraph above, to avoid any confusion and clarify that the paragraph above isn't expected to be rendered at default size.

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