[Appearance] Show Home Button too liberal with accepting URLs |
||||||||
Issue description1. Look at the Appearance Section in MD Settings 2. Make sure Show Home Button is enabled 3. Select Other 4. Enter '@@@@@@' into the URL field and press Enter. Now @@@@@@ is the Home URL. We should probably use the same URL validation logic that the On Startup section uses.
,
Feb 21 2017
FWIW, this is the case with the old Options too. I agree it should be fixed, but don't think it should be a blocker for issue 671375 .
,
Feb 21 2017
did you try about:inducebrowsercrashforrealz yet? ;)
,
Feb 28 2017
Notes from some initial investigation. 1) We are using a settings-input element for the home button text input field [1], which does not seem to currently expose any way to do validation from the outside. 2) settings-input itself does "auto validation" based on a given pattern, which we do not actually use, since we don't pass any value for "patterns", see [2]. 3) We would need to do async validation calling StartupUrlsPageBrowserProxy#validateStartupPage to achieve same behavior as the "on startup" section, see [3]. 4) The UI needs to be adjusted to accommodate the space needed for the validation error, see attachment. [1] https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/appearance_page/appearance_page.html?l=123 [2] https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/controls/settings_input.html?l=18,20 [3] https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/on_startup_page/startup_urls_page_browser_proxy.js?l=28
,
Feb 28 2017
,
Mar 4 2017
,
Mar 4 2017
,
Mar 16 2017
To add to the list of things that is wrong with this - 5) the secondary-text UI currently say "www.example.com" when the input is actually empty, which is kind of misleading and makes me think my homepage is actually set to www.example.com 6) there's no confirmation button to let the user say "ok I'm choosing this as my homepage url!", instead it just saves on pressing enter or on blur. I could see if a user just types in the url and refresh or close the window thinking they saved it.
,
Mar 16 2017
,
Mar 17 2017
dpapad@ or scottchen@: who wants to fix this?
,
Mar 21 2017
,
Mar 21 2017
dbeam@ there's yet another one: https://bugs.chromium.org/p/chromium/issues/detail?id=619768&desc=2 In there, bettes@ says the secondary text should say "Custom" when the field is empty, but tbuckley said in Issue 698611 that the text should say "New Tab page" until the user enters something in there. Which one do I go with?
,
Mar 22 2017
scottchen@: this bug is about validating the startup URL. if this affects visual presentation, that's fine, but it's not about the oddness of the placeholder in this UI.
,
Mar 22 2017
,
Mar 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f7628484099e467516f27c8a6e9f056461c1aab commit 2f7628484099e467516f27c8a6e9f056461c1aab Author: scottchen <scottchen@chromium.org> Date: Sat Mar 25 01:13:37 2017 MD Settings: validate home button url input. This CL adds validation to the home url input, and shows error message when the input is an invalid url. This CL also removes some of settings-input's logic, since it is no longer being used by anything outside of appearance-page. BUG= 681243 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2766093002 Cr-Commit-Position: refs/heads/master@{#459614} [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/app/settings_strings.grdp [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/browser/resources/settings/appearance_page/appearance_page.html [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/browser/resources/settings/appearance_page/appearance_page.js [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/browser/resources/settings/controls/settings_input.html [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/browser/resources/settings/controls/settings_input.js [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/browser/resources/settings/search_engines_page/search_engine_dialog.html [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc [modify] https://crrev.com/2f7628484099e467516f27c8a6e9f056461c1aab/chrome/test/data/webui/settings/appearance_page_test.js
,
Mar 31 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by tbuckley@google.com
, Jan 25 2017Labels: Hotlist-MD-Settings-Appearance