Bug 206783 – Web Inspector: Color picker: when zoomed, crosshair is misplaced on clicking

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

Created attachment 388742 [details]
[Video] Bug

Steps:
1. Press Command-+ twice.
2. Click in the middle of the HSB color square of the color picker.

Actual:
The circle indicating the selected color is way on the left of the mouse cursor.

Expected:
The circle indicating the selected color is right under the mouse cursor.

Notes:
This is not a regression.

[Nikita Vasilyev]

```
let point = window.webkitConvertPointFromPageToNode(this._element, new WebKitPoint(event.pageX, event.pageY));
```

webkitConvertPointFromPageToNode incorrectly interpolates the values when zoomed.

[Radar WebKit Bug Importer]

<rdar://problem/58886599>

[Nikita Vasilyev]

Created attachment 388789 [details]
Patch

[Nikita Vasilyev]

Created attachment 388790 [details]
[Video] With patch applied

[Devin Rousso]

Comment on attachment 388789 [details]
Patch

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

r-, as this breaks the bezier editor in RTL

> Source/WebInspectorUI/UserInterface/Models/Geometry.js:44
> +        return new WI.Point(

Style: we don't do this kind of multi-line function calling, so please collapse it into a single line
```
    return new WI.Point(event.pageX - rect.x, event.pageY - rect.y);
```

> Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:191
> +        let point = {

NIT: `point` isn't used anywhere else, so you could just inline it:
```
    this._setCrosshairPosition({
        x: event.pageX - rect.x,
        y: event.pageY - rect.y,
    });
```

[Nikita Vasilyev]

(In reply to Devin Rousso from comment #5)
> Comment on attachment 388789 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388789&action=review
> 
> r-, as this breaks the bezier editor in RTL

This happens because we mirror the content with CSS transforms.

body[dir=rtl] .bezier-editor > .bezier-container {
    transform: scaleX(-1);
}

I see we do this a lot but it's a bad practice from accessibility standpoint — it doesn't update the accessibility tree but reverses the order visually.

Currently, both accessibility and RTL support of WI are in a poor state, so I'll just fix one thing at a time.

[Nikita Vasilyev]

In RTL mode, input fields are in the reverse order from CSS.

In CSS: cubic-bezier(0, 0.5, 1, 0.5)
In the popover: [0.5] [1] [0.5] [0]

In RTL mode, people would have to context switch when editing values in CSS vs the popover. This is pretty confusing. In this particular case, I think we are only making interactions more confusing.

[Nikita Vasilyev]

Created attachment 390184 [details]
Patch

(In reply to Devin Rousso from comment #5)
> Comment on attachment 388789 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388789&action=review
> 
> r-, as this breaks the bezier editor in RTL

Bezier editor is strictly LTR now (Bug 206968). Resetting to "r?".

[WebKit Commit Bot]

Comment on attachment 390184 [details]
Patch

Clearing flags on attachment: 390184

Committed r256647: <https://trac.webkit.org/changeset/256647>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.