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

Fri Jul 29 07:23:25 PDT 2011

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

--- Comment #8 from Mihnea Ovidenie <mihnea at adobe.com>  2011-07-29 07:23:25 PST ---
(In reply to comment #6)
> (From update of attachment 102239 [details])
> 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".

Done.

> 
> > LayoutTests/fast/borders/border-width-percent.html:1
> > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
> 
> It's marginally better to use an HTML5 doctype.

Done.

> 
> > 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.

Done.

> 
> > 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?

For the purpose of this test, this should be ok in my opinion.
> 
> > 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.

Done. Filed https://bugs.webkit.org/show_bug.cgi?id=65375

> 
> > Source/WebCore/html/HTMLElement.cpp:143
> > +    if (!attr->value().isEmpty() && !attr->value().isNull())
> 
> This check makes no sense - null string is also empty.

Done.

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

Done.

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