[Webkit-unassigned] [Bug 65176] BORDER attribute with the object tag, using percentage values not working.

Thu Jul 28 11:46:50 PDT 2011

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

Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102239|commit-queue?               |commit-queue-
               Flag|                            |

--- Comment #6 from Alexey Proskuryakov <ap at webkit.org>  2011-07-28 11:46:49 PST ---
(From update of attachment 102239)
View in context: https://bugs.webkit.org/attachment.cgi?id=102239&action=review

I didn't attempt to think about the meaningful parts of this patch, just noting a few nitpicks about coding style.

> LayoutTests/ChangeLog:8
> +        Added the tests with border width with percentages in a new file. Removed them

It might be slightly less confusing to say "moved" instead of "removed".

> LayoutTests/fast/borders/border-width-percent.html:1
> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">

It's marginally better to use an HTML5 doctype.

> LayoutTests/fast/borders/border-width-percent.html:8
> +<p id="description">This tests the border width property with percent values for HTML elements.</p>

Extremely small nit: no need for id on this element any more.

> LayoutTests/fast/borders/border-width-percent.html:21
> +    return borderBoxWidth / 2;

Is it OK if the result is non-integer? Do we need rounding here?

> Source/WebCore/html/HTMLElement.cpp:142
> +    unsigned int borderWidth = 0;

We don't say "unsigned int" as a matter of coding style. Please just use "unsigned", and it would be nice to file a bug against style checker.

> Source/WebCore/html/HTMLElement.cpp:143
> +    if (!attr->value().isEmpty() && !attr->value().isNull())

This check makes no sense - null string is also empty.

> Source/WebCore/html/HTMLElement.h:93
> +    unsigned int parseBorderWidthAttribute(Attribute*);

Just unsigned.

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