Bug 204874 – [WinCairo] Improve Inspectable Target Page to adapt a long title and URL

• Component: Web Inspector
• Status: RESOLVED FIXED

[Yousuke Kimoto]

When a target content which has long URL or long title and those lengths exceed the width of the list of inspectable webviews, an "inspect" button is placed outside of the web inspector's visible area. It make users irritate since they need to scroll to seek "Inspect" button at the most right of the page.

[Yousuke Kimoto]

Created attachment 384881 [details]
patch

[Fujii Hironori]

Comment on attachment 384881 [details]
patch

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

> Source/WebKit/UIProcess/socket/RemoteInspectorProtocolHandler.cpp:160
> +        "  table, td { border: 1px solid #d3d7cf; border-left: none; border-right: none; table-layout: fixed;}"

I think 'table-layout' should apply only to table element. Why do you want to apply it to td element?

> Source/WebKit/UIProcess/socket/RemoteInspectorProtocolHandler.cpp:165
> +        "  .targeturl { width: auto; color: #babdb6; overflow: hidden; background: #eee; word-wrap: break-word; overflow-wrap: break-word; }"

Why 'width: auto' is needed? Its initial value is 'auto'.
Why 'overflow: hidden' is needed? You are using ellipsis and break-word. Can it overflow?

[Yousuke Kimoto]

(In reply to Fujii Hironori from comment #2)
> > Source/WebKit/UIProcess/socket/RemoteInspectorProtocolHandler.cpp:160
> > +        "  table, td { border: 1px solid #d3d7cf; border-left: none; border-right: none; table-layout: fixed;}"
> 
> I think 'table-layout' should apply only to table element. Why do you want
> to apply it to td element?

Thanks, the attribute should be applied in table element..
 
> > Source/WebKit/UIProcess/socket/RemoteInspectorProtocolHandler.cpp:165
> > +        "  .targeturl { width: auto; color: #babdb6; overflow: hidden; background: #eee; word-wrap: break-word; overflow-wrap: break-word; }"
> 
> Why 'width: auto' is needed? Its initial value is 'auto'.
> Why 'overflow: hidden' is needed? You are using ellipsis and break-word. Can
> it overflow?

'width: auto' is unnecessary. I'll delete them.

[Yousuke Kimoto]

Created attachment 384886 [details]
patch

[Fujii Hironori]

Why 'overflow: hidden' is needed? You are using ellipsis and break-word. Can it overflow?

[Yousuke Kimoto]

(In reply to Fujii Hironori from comment #5)
> Why 'overflow: hidden' is needed? You are using ellipsis and break-word. Can
> it overflow?

Sorry, I missed your comment. It's also unnecessary.

[Yousuke Kimoto]

Created attachment 384894 [details]
patch

[Fujii Hironori]

Comment on attachment 384894 [details]
patch

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

> Source/WebKit/UIProcess/socket/RemoteInspectorProtocolHandler.cpp:164
> +        "  .targetname { font-weight: bold; overflow: hidden; white-space:nowrap; text-overflow: ellipsis; }"

Do you need this 'overflow: hidden'? Can it overflow? Even though ellipsis is used.

[Fujii Hironori]

Comment on attachment 384894 [details]
patch

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

>> Source/WebKit/UIProcess/socket/RemoteInspectorProtocolHandler.cpp:164
>> +        "  .targetname { font-weight: bold; overflow: hidden; white-space:nowrap; text-overflow: ellipsis; }"
> 
> Do you need this 'overflow: hidden'? Can it overflow? Even though ellipsis is used.

Ah, I misunderstood. It is needed.

[Fujii Hironori]

Comment on attachment 384894 [details]
patch

LGTM

[Yousuke Kimoto]

Thank you for your review.

[WebKit Commit Bot]

Comment on attachment 384894 [details]
patch

Clearing flags on attachment: 384894

Committed r253199: <https://trac.webkit.org/changeset/253199>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/57685185>

[Yousuke Kimoto]

Memo:
>>> +        "  .targetname { font-weight: bold; overflow: hidden; white-space:nowrap; text-overflow: ellipsis; }"
>> 
>> Do you need this 'overflow: hidden'? Can it overflow? Even though ellipsis is used.
> 
>Ah, I misunderstood. It is needed.
I should have explained the latter case. The following page explains why 'overflow: hidden' is required.

https://developer.mozilla.org/en-US/docs/Web/CSS/text-overflow
>The text-overflow property doesn't force an overflow to occur. To make text overflow its container you have to set other CSS properties: overflow and white-space.