Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2966413002:
Show entire URLs instead of horizontal scrollbars in about:media-internals (Closed)
|
Created:
3 years, 5 months ago by François Beaufort Modified:
3 years, 4 months ago CC:
chromium-reviews, darin-cc_chromium.org, feature-media-reviews_chromium.org, jam Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionShow entire URLs instead of horizontal scrollbars in about:media-internals
BUG=739711
Review-Url: https://codereview.chromium.org/2966413002
Cr-Commit-Position: refs/heads/master@{#495496}
Committed: https://chromium.googlesource.com/chromium/src/+/7cc9de4b71f020612213af96e08d5d2e160cab28
Patch Set 1 #
Total comments: 1
Patch Set 2 : Rebase #Messages
Total messages: 19 (10 generated)
beaufort.francois@gmail.com changed reviewers: + dalecurtis@google.com
Hello Dale, What do you think of this simple CSS fix?
Description was changed from ========== Show entire URLs instead of horizontal scrollbars in about:media-internals BUG=739711 ========== to ========== Show entire URLs instead of horizontal scrollbars in about:media-internals BUG=739711 ==========
beaufort.francois@gmail.com changed reviewers: + dalecurtis@chromium.org - dalecurtis@google.com
dalecurtis@chromium.org changed reviewers: + sandersd@chromium.org
lgtm! sandersd@ is working on changing the layout a bit in https://codereview.chromium.org/2964713004 but this will still be a good addition.
On 2017/07/06 16:50:08, DaleCurtis wrote: > lgtm! sandersd@ is working on changing the layout a bit in > https://codereview.chromium.org/2964713004 but this will still be a good > addition. I'm not so sure about this change, in particular how it interacts with https://codereview.chromium.org/2964713004/. I'd rather go the other way (disable line breaks) and continue to rely on scrolling. François: Do you have a use case for always seeing the whole URL without scrolling, assuming that my CL is merged?
On 2017/07/06 17:28:25, sandersd (OOO until July 31) wrote: > On 2017/07/06 16:50:08, DaleCurtis wrote: > > lgtm! sandersd@ is working on changing the layout a bit in > > https://codereview.chromium.org/2964713004 but this will still be a good > > addition. > > I'm not so sure about this change, in particular how it interacts with > https://codereview.chromium.org/2964713004/. > > I'd rather go the other way (disable line breaks) and continue to rely on > scrolling. > > François: Do you have a use case for always seeing the whole URL without > scrolling, assuming that my CL is merged? I've tried my CSS change with your CL and it works fine. I've found myself looking a lot at the end of the URL when inspecting content. Your change helps a lot as well.
> I've tried my CSS change with your CL and it works fine. I've found myself > looking a lot at the end of the URL when inspecting content. Makes sense, I'm willing to give it a go. lgtm. https://codereview.chromium.org/2966413002/diff/1/content/browser/resources/m... File content/browser/resources/media/media_internals.css (right): https://codereview.chromium.org/2966413002/diff/1/content/browser/resources/m... content/browser/resources/media/media_internals.css:185: word-break: break-all; Are you sure that we want this in all selectable buttons, and not just players? (The blue colors change was applied to all, so the answer is probably yes.)
The CQ bit was checked by beaufort.francois@gmail.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by beaufort.francois@gmail.com
The patchset sent to the CQ was uploaded after l-g-t-m from dalecurtis@chromium.org, sandersd@chromium.org Link to the patchset: https://codereview.chromium.org/2966413002/#ps20001 (title: "Rebase")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1503040655055380,
"parent_rev": "b3291682252d6fa56640e7350609d0a0c380090d", "commit_rev":
"7cc9de4b71f020612213af96e08d5d2e160cab28"}
Message was sent while issue was closed.
Description was changed from ========== Show entire URLs instead of horizontal scrollbars in about:media-internals BUG=739711 ========== to ========== Show entire URLs instead of horizontal scrollbars in about:media-internals BUG=739711 Review-Url: https://codereview.chromium.org/2966413002 Cr-Commit-Position: refs/heads/master@{#495496} Committed: https://chromium.googlesource.com/chromium/src/+/7cc9de4b71f020612213af96e08d... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/7cc9de4b71f020612213af96e08d... |
