Bug 169765 - Web Inspector: RTL: minor layout issues in Breakpoint Editor popover
Summary: Web Inspector: RTL: minor layout issues in Breakpoint Editor popover
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-16 10:34 PDT by BJ Burg
Modified: 2017-03-20 20:27 PDT (History)
5 users (show)

See Also:


Attachments
[RTL] screenshot (133.94 KB, image/png)
2017-03-16 10:34 PDT, BJ Burg
no flags Details
Patch (6.78 KB, patch)
2017-03-16 12:37 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
[image] After Patch is applied (104.43 KB, image/png)
2017-03-16 12:37 PDT, Devin Rousso
no flags Details
Patch (8.17 KB, patch)
2017-03-18 18:12 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
[Image] After Patch is applied (96.96 KB, image/png)
2017-03-18 18:12 PDT, Devin Rousso
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2017-03-16 10:34:28 PDT
Created attachment 304652 [details]
[RTL] screenshot

- There is no padding between some inputs and labels
- The help text for log statements has flipped braces. This is a Webcore bug we may need to work around.
- text input fields for log messages, etc should be forced LTR.
Comment 1 Devin Rousso 2017-03-16 12:37:17 PDT
Created attachment 304675 [details]
Patch
Comment 2 Devin Rousso 2017-03-16 12:37:35 PDT
Created attachment 304676 [details]
[image] After Patch is applied
Comment 3 Matt Baker 2017-03-16 13:12:26 PDT
Is it possible to align the Condition placeholder as well? The rest looks good (except for the issue with the expression hint, but that's a text rendering bug AFAIK).
Comment 4 Devin Rousso 2017-03-16 13:22:36 PDT
(In reply to comment #3)
> Is it possible to align the Condition placeholder as well? The rest looks
> good (except for the issue with the expression hint, but that's a text
> rendering bug AFAIK).

I think we set every CodeMirror instance to RTL due to some issues with they way it renders scrollbars.
Comment 5 BJ Burg 2017-03-18 16:55:07 PDT
(In reply to comment #3)
> Is it possible to align the Condition placeholder as well? The rest looks
> good (except for the issue with the expression hint, but that's a text
> rendering bug AFAIK).

For the latter issue, this can be fixed by setting dir=ltr and using text-align:right for body[dir=rtl]. The string is localized, but will still render wrong because curly braces get swapped for some reason.
Comment 6 BJ Burg 2017-03-18 16:56:31 PDT
Comment on attachment 304675 [details]
Patch

r- because we need to force LTR for the action fields (log message ,evaluate script, etc). As is, the hint text is wrong and the field doesn't match the condition editor which is codemirror-backed.
Comment 7 Devin Rousso 2017-03-18 18:12:00 PDT
Created attachment 304890 [details]
Patch
Comment 8 Devin Rousso 2017-03-18 18:12:26 PDT
Created attachment 304891 [details]
[Image] After Patch is applied
Comment 9 BJ Burg 2017-03-20 19:26:38 PDT
Comment on attachment 304890 [details]
Patch

r=me
Comment 10 WebKit Commit Bot 2017-03-20 20:27:51 PDT
Comment on attachment 304890 [details]
Patch

Clearing flags on attachment: 304890

Committed r214210: <http://trac.webkit.org/changeset/214210>
Comment 11 WebKit Commit Bot 2017-03-20 20:27:56 PDT
All reviewed patches have been landed.  Closing bug.