[webkit-reviews] review denied: [Bug 217447] Web Inspector: update styles to use CSS properties with neutral directionality : [Attachment 412052] Update styles to use CSS properties with neutral directionality

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 21 17:42:42 PDT 2020


Devin Rousso <drousso at apple.com> has denied Federico Bucchi
<fbucchi at apple.com>'s request for review:
Bug 217447: Web Inspector: update styles to use CSS properties with neutral
directionality
https://bugs.webkit.org/show_bug.cgi?id=217447

Attachment 412052: Update styles to use CSS properties with neutral
directionality

https://bugs.webkit.org/attachment.cgi?id=412052&action=review




--- Comment #4 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 412052
  --> https://bugs.webkit.org/attachment.cgi?id=412052
Update styles to use CSS properties with neutral directionality

View in context: https://bugs.webkit.org/attachment.cgi?id=412052&action=review

r- for the reasons below

I think you're missing a lot of other `body[dir=*]` rules :(

I'd also suggest updating `-webkit-margin-*`/`-webkit-padding-*` to use
`margin-inline-*`/`padding-inline-*` instead

> Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.css:80
> +    float: inline-start;

Safari doesn't support `inline-*` for `float`

ditto in other files

> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.css:119
>      border-right: var(--cpu-timeline-view-overview-divider-border-end);

Why are we keeping this?

> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.css:120
> +    border-block-end: var(--cpu-timeline-view-overview-divider-border-end);

This should be `-inline-` since it's left/right (we only support a horizontal
writing mode, so `-inline-` will always be left/right and `-block-` will always
be top/bottom).

Also, please remove the variable and inline its value now that it's only used
in one place.

ditto in other files

> Source/WebInspectorUI/UserInterface/Views/FindBanner.css:100
> +    border-start-start-radius:
var(--find-banner-search-box-border-radius-start);
> +    border-end-start-radius:
var(--find-banner-search-box-border-radius-start);
> +    border-start-end-radius:
var(--find-banner-search-box-border-radius-end);
> +    border-end-end-radius: var(--find-banner-search-box-border-radius-end);

??? these are not CSS properties

ditto in other files


More information about the webkit-reviews mailing list