Network error intersititlals titles clean up |
||||||||||||||||||||
Issue descriptionSimplify the page titles to just show the host name. Removing the path and protocol. NetErrors + Titles: https://docs.google.com/presentation/d/1yh6_IeBqK927s8Dy5z75fyAVehkQaMh8HfMFAztsvJE/edit#slide=id.g1296ab7fb8_0_21 - using the hostname as the title is a good move away from scary syntax - desaturating the favicon is OK (but not necessary) Eng: edwardjung@ PM: talo@ Are you planning on experimenting before launch? No Any new strings? No Do the existing perf tests exercise all aspects of your new feature(s)? Yes
,
Aug 1 2016
I haven't got my head round how to pass the error state to the tab view, but I have been experimenting with the saturation of the favicon. Just to clarify, did we agreeing on greyscale or desaturation of the icon? For the standard page icon, there isn't much noticeable difference. Also was the title going to be dimmed too?
,
Aug 1 2016
In the mocks, I fully desaturated the icons which made them greyscale - but I defer to your judgement. Nope - no title dimming - keep the normal color(s). I was hoping we could swap out the title in the HTML for the neterrors to show foo.com (instead of the "https://foo.com/home.php is not available" string).
,
Aug 1 2016
@mmenke, I was wondering if you had any pointers regarding the neterror page loading process passing a status to the tab view to indicate the page has a network error? I'm currently looking at c/b/ui/views/tabs/TabRendererData which contains various tab statuses like loading, crashed etc. c/b/ui/views/tabs/Tab has the tab rendering logic and I was going to check for a network error status. Just not sure if the neterror page triggers a tab data update. I assume it does. Any pointers would be appreciated. Thanks.
,
Aug 1 2016
I don't understand what you're asking me.
,
Aug 1 2016
Re 5: We want to desaturate the favicon in the tab for tabs showing any of the network error pages. So more simply would there be a way to check if the page is currently showing a network error from the Tab class. I was thinking of setting a flag within TabRendererData when a network error page is shown.
,
Aug 1 2016
,
Aug 1 2016
WebContents knows if a page is an error page - see NavigationEntry. NavigationHandle also has GetNetErrorCode, but don't know what would happen there when we show error pages for HTTP errors (Could actually be neither place handles that properly). I'm unfamiliar with any of the views/ classes, or how they're hooked up, but WebContents (Or some chrome/ class that knows about it) can presumably pass that information over to it.
,
Aug 5 2016
Re#9: Thanks mmenke, that was exactly the pointer I needed. I apply a greyscale filter with .8 opacity in the screenshots attached. Tried it against various themes. Not sure applying it to the default page icon does anything. How does this look to you all? I'll do a Windows build and see what it looks like. Mac uses a different view to render tabs with Cocoa so that will be done separately.
,
Aug 5 2016
A question was raised as what title to show for non https / http pages like files or chrome internal pages. I was going to show the file name, since there is no 'host' and the full path could be very long (which is currently what we show).
,
Aug 5 2016
Note that we don't always have a file name, and if we do, it isn't always meaningful. "file:///", for instance, or "chrome://chickens/are/tasty/_#?"
,
Aug 5 2016
re: #11 Nit from the screenshots - I'd like to show just the hostname (and no the scheme or path or port) - so instead of https://www.facebook.com/xyz it'd just be www.facebook.com I think we're already doing something like the the clank add-to-homescreen banners. Your grayscale + opacity works for me. Maybe you could run it past bettes@ as a sanity check?
,
Aug 5 2016
Re:#14 - Sorry forgot to to mention ignore the titles, the title change I'm doing separate currently under review. re:#13 - Good point. Seems like showing the full address would be the best option, so as not to have url.host() extract something random back.
,
Aug 5 2016
You could imagine special rules for each scheme: For file URLs, just display the full path without the file://, for chrome URLs, just display the scheme and host, for extensions, display the extension name, if we can get it (We can't, without a lot more plumbing), etc. Think there's very little benefit for special logic for anything but HTTP, but others may disagree.
,
Aug 5 2016
Windows build screenshots. This time with the host name only for the tab title.
,
Aug 9 2016
Followed up with Hwi and bettes. + Bettes was okay with the desaturation but only wanted it for the focused tab. In general it would be the case you encounter an error in the current tab. However there is a case where if you restart Chrome and you are offline your tabs are all offline with desaturated favicons. + Hwi didn't have accessibility concerns but did think the tab looked disabled. More so for unfocused tabs. Also felt the saturation change was too subtle for users to notice. Thoughts?
,
Aug 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e76b58098fd472f255d3d9eda75a95ccbfce9abc commit e76b58098fd472f255d3d9eda75a95ccbfce9abc Author: edwardjung <edwardjung@chromium.org> Date: Thu Aug 11 13:38:48 2016 Change network error titles to just the hostname BUG= 630190 Review-Url: https://codereview.chromium.org/2214393003 Cr-Commit-Position: refs/heads/master@{#411315} [modify] https://crrev.com/e76b58098fd472f255d3d9eda75a95ccbfce9abc/chrome/browser/content_settings/content_settings_browsertest.cc [modify] https://crrev.com/e76b58098fd472f255d3d9eda75a95ccbfce9abc/chrome/browser/policy/policy_browsertest.cc [modify] https://crrev.com/e76b58098fd472f255d3d9eda75a95ccbfce9abc/components/error_page/common/localized_error.cc [modify] https://crrev.com/e76b58098fd472f255d3d9eda75a95ccbfce9abc/components/error_page_strings.grdp
,
Aug 11 2016
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/49bd514cfcc39a8408c9cc383d62d60f43ee0e98 commit 49bd514cfcc39a8408c9cc383d62d60f43ee0e98 Author: jyquinn <jyquinn@google.com> Date: Thu Aug 11 21:36:24 2016
,
Aug 12 2016
Update having spoken to ainslie offline regarding hwi's and bettes comments. + As nothing else in the tab styling is changing, the tabs shouldn't look completely disabled. In most cases the net error would be in the foreground tab. + For inactive tabs a user would be able to figure out the tab isn't disabled when they attempt to click on the tab. This scenario should only be when you are offline and restarting Chrome, reloading previous tabs. The active tab will also be in the same state. Decision is to show desaturated icons on desktop first.
,
Aug 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0fb4518d83764eb357d3e043c529ae6aeff99ff9 commit 0fb4518d83764eb357d3e043c529ae6aeff99ff9 Author: edwardjung <edwardjung@chromium.org> Date: Mon Aug 15 12:04:14 2016 Desaturate the favicon shown in the tab when a network error is encountered. BUG= 630190 Review-Url: https://codereview.chromium.org/2217273002 Cr-Commit-Position: refs/heads/master@{#411956} [modify] https://crrev.com/0fb4518d83764eb357d3e043c529ae6aeff99ff9/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc [modify] https://crrev.com/0fb4518d83764eb357d3e043c529ae6aeff99ff9/chrome/browser/ui/views/tabs/tab.cc [modify] https://crrev.com/0fb4518d83764eb357d3e043c529ae6aeff99ff9/chrome/browser/ui/views/tabs/tab_renderer_data.h
,
Sep 6 2016
,
Sep 22 2016
Mac implementation is now in Canary. @spqchan Thanks for the Mac implementation and refactor of my code. All desktop platforms should now be aligned. On Chrome for Android the host name only page titles has now filtered down. |
||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||
Comment 1 by edwardjung@chromium.org
, Jul 21 2016