Summary: | [WinCairo] Improve Inspectable Target Page to adapt a long title and URL | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yousuke Kimoto <Yousuke.Kimoto> | ||||||||
Component: | Web Inspector | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, Hironori.Fujii, inspector-bugzilla-changes, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Yousuke Kimoto
2019-12-04 23:08:03 PST
Created attachment 384881 [details]
patch
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? (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. Created attachment 384886 [details]
patch
Why 'overflow: hidden' is needed? You are using ellipsis and break-word. Can it overflow? (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. Created attachment 384894 [details]
patch
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. 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. Comment on attachment 384894 [details]
patch
LGTM
Thank you for your review. Comment on attachment 384894 [details] patch Clearing flags on attachment: 384894 Committed r253199: <https://trac.webkit.org/changeset/253199> All reviewed patches have been landed. Closing bug. 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. |