[Webkit-unassigned] [Bug 58608] Groove/inset/outset borders show solid if the color is black

[bugzilla-daemon at webkit.org]

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

Adenilson Cavalcanti Silva <savagobr at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |savagobr at yahoo.com

--- Comment #54 from Adenilson Cavalcanti Silva <savagobr at yahoo.com> ---
Simon

Thanks for reviewing the patch. I will comment inline.

> Does this patch fix the table rendering issues that the earlier patch broke?
> 

>From the old messages, it seems that CSS1 was failing. I just attached the output for those tests and all is good. 

I also tested (fast/css, fast/table, fast/borders) and there are no regressions.


> 
> Why did you add this here? It's just adding technical debt that someone has
> to clean up later.
> 

I'm a bit unfamiliar with new tests when they aren't reftests. Should I remove it from TestExpectations?


> > LayoutTests/fast/borders/mixed-border-style2.html:7
> > +      -moz-box-sizing: border-box;
> 
> You can remove this.
> 

Coolio, will upload a new patch addressing this.



> 
> Where did these magic numbers come from?
> 

They came from the original patch, I tried other values but those seems to work better.


> > Source/WebCore/rendering/RenderObject.cpp:2455
> > +    Operation operation = (side == BSTop || side == BSLeft) == (style == INSET) ?
> > +        Darken : Lighten;
> 
> This line doesn't need to wrap.
> 

All in a single line then? Ok.



> 
> Would be nice to add a comment justifying this behavior.

What about '// Here we will darken the border decoration color when needed. This will yield a similar behavior as in FF.' ?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141201/58c7eae5/attachment-0001.html>