[webkit-reviews] review denied: [Bug 84235] Web Inspector: Does not have search navigation button for going through matches in either direction (prev, next) : [Attachment 138043] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied sam <dsam2912 at gmail.com>'s
request for review:
Bug 84235: Web Inspector: Does not have search navigation button for going
through matches in either direction (prev, next)
https://bugs.webkit.org/show_bug.cgi?id=84235

Attachment 138043: Patch
https://bugs.webkit.org/attachment.cgi?id=138043&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=138043&action=review


This is almost ready, few nits left.

> Source/WebCore/inspector/front-end/SearchController.js:344
> +		case "prev":

As I mentioned, case should be below switch, no indentation.

> Source/WebCore/inspector/front-end/inspector.css:279
> +    -webkit-border-image: none;

Remove this.

> Source/WebCore/inspector/front-end/inspector.css:280
> +    border: 1px solid rgba(120, 120, 120, 0.4);

On my box, it looks like there is a gray border by default that vanishes on
hover. I'd suggest to make this one transparent and move the definition into
the :hover.

> Source/WebCore/inspector/front-end/inspector.css:290
> +    -webkit-border-image: none;

Remove this.

> Source/WebCore/inspector/front-end/inspector.css:302
> +    background-image: url('Images/searchPrev.png');

We do not quote paths in url()

> Source/WebCore/inspector/front-end/inspector.css:306
> +    background-image: url('Images/searchNext.png');

ditto