Bug 170292 – Web Inspector: RTL: results in Search navigation sidebar have misplaced highlights

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

[Blaze Burg]

This triggers pathologically slow complex text layout, which I filed a separate bug about. The fix for this works around the perf issue.

[Blaze Burg]

Created attachment 305879 [details]
Patch

[Blaze Burg]

Created attachment 305880 [details]
Before [RTL]

[Blaze Burg]

Created attachment 305881 [details]
after [RTL]

[Blaze Burg]

Comment on attachment 305879 [details]
Patch

Found an issue when the result line is too big.

[Blaze Burg]

Created attachment 305886 [details]
Patch

[Matt Baker]

Comment on attachment 305886 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/SearchIcons.css:50
> +.source-code-match .icon {

Nice cleanup, so tired of seeing .yet-another-icon .icon everywhere.

> Source/WebInspectorUI/UserInterface/Views/SearchResultTreeElement.js:42
> +        const charactersToShowBeforeSearchMatch = isRTL ? 20 : 15;

Why is this changing to 20? It feels wrong presenting less/different data in RTL.

> Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.css:57
> +    text-align: right;

This looks wrong. Search results are now right aligned when viewed in LTR.

[Blaze Burg]

(In reply to Matt Baker from comment #6)
> Comment on attachment 305886 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=305886&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/SearchIcons.css:50
> > +.source-code-match .icon {
> 
> Nice cleanup, so tired of seeing .yet-another-icon .icon everywhere.
> 
> > Source/WebInspectorUI/UserInterface/Views/SearchResultTreeElement.js:42
> > +        const charactersToShowBeforeSearchMatch = isRTL ? 20 : 15;
>
> Why is this changing to 20? It feels wrong presenting less/different data in
> RTL.

We have to be more conservative so the highlight text doesn't run off into overflow. It's hard to determine what will actually show in RTL since we don't know how much text might overflow to the left, whereas in LTR we know exactly what text starts on the left side. This value worked for me when resizing the sidebar to the minimum expanded width.
 
> > Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.css:57
> > +    text-align: right;
> 
> This looks wrong. Search results are now right aligned when viewed in LTR.

Will fix.

I'm tempted to just leave subtitles out since we can't find a good way to overflow them without making search results impossible to consistently align left. If the tooltip still displays the title and subtitle, that might be fine.

[Blaze Burg]

Created attachment 306058 [details]
Patch v1.1

[Blaze Burg]

Created attachment 306059 [details]
search results - LTR

[Blaze Burg]

Created attachment 306060 [details]
search results - RTL

[Blaze Burg]

(In reply to Brian Burg from comment #7)
>
> I'm tempted to just leave subtitles out since we can't find a good way to
> overflow them without making search results impossible to consistently align
> left. If the tooltip still displays the title and subtitle, that might be
> fine.

Oops, this comment belongs in a different bug.

[WebKit Commit Bot]

Comment on attachment 306058 [details]
Patch v1.1

Clearing flags on attachment: 306058

Committed r214864: <http://trac.webkit.org/changeset/214864>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.