Bug 171954 – Web Inspector: RTL: "Elements > Styles - Rules" rule and media query headers should be left aligned

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Created attachment 309658 [details]
[Image] Bug

rdar://problem/31961976

Since the CSS source is strongly LTR, it doesn't make sense to flip CSS selectors.

[Blaze Burg]

<rdar://problem/31961976>

[Nikita Vasilyev]

Created attachment 309665 [details]
Patch

[Nikita Vasilyev]

Created attachment 309666 [details]
[Image] With patch applied

[Matt Baker]

Comment on attachment 309665 [details]
Patch

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

r=me, with comment

> Source/WebInspectorUI/UserInterface/Views/StyleDetailsPanel.js:38
> +        this.element.dir = "ltr";

I don't understand why this is needed. Commenting it out doesn't break anything, as far as I can tell.

[Nikita Vasilyev]

Created attachment 309674 [details]
[Animated GIF] Focusing on selector without dir=ltr

(In reply to Matt Baker from comment #4)
> Comment on attachment 309665 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309665&action=review
> 
> r=me, with comment
> 
> > Source/WebInspectorUI/UserInterface/Views/StyleDetailsPanel.js:38
> > +        this.element.dir = "ltr";
> 
> I don't understand why this is needed. Commenting it out doesn't break
> anything, as far as I can tell.

Try editing a selector. I left a comment in the changelog.

[Matt Baker]

Comment on attachment 309665 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Views/StyleDetailsPanel.js:38
>>> +        this.element.dir = "ltr";
>> 
>> I don't understand why this is needed. Commenting it out doesn't break anything, as far as I can tell.
> 
> Try editing a selector. I left a comment in the changelog.

Ah, okay. I forgot selectors could be edited!

I think the direction only needs to be set for the selector input (since WebInspector.CodeMirrorEditor already forces dir="ltr"). Maybe it should move to CSSStyleDeclarationSection.js:66, where the <textarea> is created:

if (this.selectorEditable) {
    this._selectorInput = this._headerElement.createChild("textarea");
    this._selectorInput.setAttribute("dir", "ltr");
    this._selectorInput.spellcheck = false;
    ...
}

[Nikita Vasilyev]

Created attachment 309729 [details]
Patch

(In reply to Matt Baker from comment #6)
> Comment on attachment 309665 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309665&action=review
> 
> >>> Source/WebInspectorUI/UserInterface/Views/StyleDetailsPanel.js:38
> >>> +        this.element.dir = "ltr";
> >> 
> >> I don't understand why this is needed. Commenting it out doesn't break anything, as far as I can tell.
> > 
> > Try editing a selector. I left a comment in the changelog.
> 
> Ah, okay. I forgot selectors could be edited!
> 
> I think the direction only needs to be set for the selector input (since
> WebInspector.CodeMirrorEditor already forces dir="ltr"). Maybe it should
> move to CSSStyleDeclarationSection.js:66, where the <textarea> is created:
> 
> if (this.selectorEditable) {
>     this._selectorInput = this._headerElement.createChild("textarea");
>     this._selectorInput.setAttribute("dir", "ltr");
>     this._selectorInput.spellcheck = false;
>     ...
> }

I guess it's unlikely that we will add any inputs or textareas to StyleDetailsPanel any time soon, so it's fine with me.

[Nikita Vasilyev]

Comment on attachment 309729 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:68
>              this._selectorInput.spellcheck = false;
> +            this._selectorInput.dir = "ltr";
>              this._selectorInput.tabIndex = -1;

I used `dir = "ltr"` instead of setAttribute because it's shorter. Also, we already use the setter syntax for spellcheck and tabIndex attributes.

[Matt Baker]

Looks good!

[WebKit Commit Bot]

Comment on attachment 309729 [details]
Patch

Clearing flags on attachment: 309729

Committed r216692: <http://trac.webkit.org/changeset/216692>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.