New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 681243 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 671375



Sign in to add a comment

[Appearance] Show Home Button too liberal with accepting URLs

Project Member Reported by tommycli@chromium.org, Jan 14 2017

Issue description

1. 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.
 

Comment 1 by tbuckley@google.com, Jan 25 2017

Blocking: 671375
Labels: Hotlist-MD-Settings-Appearance

Comment 2 by dpa...@chromium.org, Feb 21 2017

Cc: dbeam@chromium.org
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 .

Comment 3 by dbeam@chromium.org, Feb 21 2017

did you try about:inducebrowsercrashforrealz yet? ;)

Comment 4 by dpa...@chromium.org, 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
home_page_validation.png
29.5 KB View Download

Comment 5 by dbeam@chromium.org, Feb 28 2017

Cc: dpa...@chromium.org
Status: Available
Blocking: -671375

Comment 7 by dbeam@chromium.org, Mar 4 2017

Blocking: 671375
nah, this is a desktop blocker
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.
Screen Shot 2017-03-16 at 2.34.38 PM.png
37.1 KB View Download

Comment 9 Deleted

Comment 11 Deleted

Comment 12 by dbeam@chromium.org, Mar 17 2017

Labels: M-59
dpapad@ or scottchen@: who wants to fix this?
Owner: scottchen@chromium.org
Status: Assigned (was: Available)
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?

Comment 15 by dbeam@chromium.org, 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.
Status: Started (was: Assigned)
Project Member

Comment 17 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)

Sign in to add a comment