New issue
Advanced search Search tips

Issue 882594 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Better error message for invalid URL

Project Member Reported by zentaro@chromium.org, Sep 10

Issue description

Enter a garbage URL in the mount url. Click Add.

We just give a "The share could not be found on the network" error. We should know from SmbUrl that this isn't even a valid URL and display a different error to alert the user.
 
Also as a follow up, check that the path part of the URL only has one part.

Since we also have a different bug that won't land until 71 about nested shares, we should give an explicit error message for now.
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7deb467d04356ff2c43efa81f083e7ca8e4b4850

commit 7deb467d04356ff2c43efa81f083e7ca8e4b4850
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 19 00:49:06 2018

Add invalid URL error message to SMB mount shares

- Adds an error message for when an invalid SMB mount url is provided.

BUG= chromium:882594 

Change-Id: I9952a11cdaf4baa727076737636a1346c0048bc2
Reviewed-on: https://chromium-review.googlesource.com/1217852
Commit-Queue: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592287}
[modify] https://crrev.com/7deb467d04356ff2c43efa81f083e7ca8e4b4850/chrome/app/settings_strings.grdp
[modify] https://crrev.com/7deb467d04356ff2c43efa81f083e7ca8e4b4850/chrome/browser/chromeos/smb_client/smb_errors.cc
[modify] https://crrev.com/7deb467d04356ff2c43efa81f083e7ca8e4b4850/chrome/browser/chromeos/smb_client/smb_errors.h
[modify] https://crrev.com/7deb467d04356ff2c43efa81f083e7ca8e4b4850/chrome/browser/chromeos/smb_client/smb_errors_unittest.cc
[modify] https://crrev.com/7deb467d04356ff2c43efa81f083e7ca8e4b4850/chrome/browser/resources/settings/downloads_page/smb_browser_proxy.js
[modify] https://crrev.com/7deb467d04356ff2c43efa81f083e7ca8e4b4850/chrome/browser/resources/settings/downloads_page/smb_shares_page.js
[modify] https://crrev.com/7deb467d04356ff2c43efa81f083e7ca8e4b4850/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
I was able to verify this in M-71, the "Invalid URL format" error message is displayed. However in M-70, the error message is still old (screenshots attached). Looks like merge to M-70 required?

Chrome OS: 11097.0.0
Chrome: 71.0.3558.0
Device: Whitetip

Chrome OS: 11021.27.0
Chrome: 70.0.3538.32
Device: Santa
M-71.png
137 KB View Download
M-70.png
92.7 KB View Download
Labels: Merge-Request-70
Cc: zentaro@chromium.org
Project Member

Comment 8 by sheriffbot@chromium.org, Sep 26

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: There is .grd file changes and we are only 19 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-70 Merge-Approved-70
This has a merge conflict. I need to manually merge today.
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 28

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea4527721628d40c730231fe12c96cb9dc8f3181

commit ea4527721628d40c730231fe12c96cb9dc8f3181
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Sep 28 17:15:55 2018

Add invalid URL error message to SMB mount shares

- Adds an error message for when an invalid SMB mount url is provided.

BUG= chromium:882594 

(cherry picked from commit 7deb467d04356ff2c43efa81f083e7ca8e4b4850)

Change-Id: I9952a11cdaf4baa727076737636a1346c0048bc2
Reviewed-on: https://chromium-review.googlesource.com/1217852
Commit-Queue: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#592287}
Reviewed-on: https://chromium-review.googlesource.com/1251981
Cr-Commit-Position: refs/branch-heads/3538@{#735}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/ea4527721628d40c730231fe12c96cb9dc8f3181/chrome/app/settings_strings.grdp
[modify] https://crrev.com/ea4527721628d40c730231fe12c96cb9dc8f3181/chrome/browser/chromeos/smb_client/smb_errors.cc
[modify] https://crrev.com/ea4527721628d40c730231fe12c96cb9dc8f3181/chrome/browser/chromeos/smb_client/smb_errors.h
[modify] https://crrev.com/ea4527721628d40c730231fe12c96cb9dc8f3181/chrome/browser/chromeos/smb_client/smb_errors_unittest.cc
[modify] https://crrev.com/ea4527721628d40c730231fe12c96cb9dc8f3181/chrome/browser/resources/settings/downloads_page/smb_browser_proxy.js
[modify] https://crrev.com/ea4527721628d40c730231fe12c96cb9dc8f3181/chrome/browser/resources/settings/downloads_page/smb_shares_page.js
[modify] https://crrev.com/ea4527721628d40c730231fe12c96cb9dc8f3181/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ea4527721628d40c730231fe12c96cb9dc8f3181

Commit: ea4527721628d40c730231fe12c96cb9dc8f3181
Author: zentaro@chromium.org
Commiter: zentaro@chromium.org
Date: 2018-09-28 17:15:55 +0000 UTC

Add invalid URL error message to SMB mount shares

- Adds an error message for when an invalid SMB mount url is provided.

BUG= chromium:882594 

(cherry picked from commit 7deb467d04356ff2c43efa81f083e7ca8e4b4850)

Change-Id: I9952a11cdaf4baa727076737636a1346c0048bc2
Reviewed-on: https://chromium-review.googlesource.com/1217852
Commit-Queue: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#592287}
Reviewed-on: https://chromium-review.googlesource.com/1251981
Cr-Commit-Position: refs/branch-heads/3538@{#735}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Verified fixed in M-70, the "Invalid URL format" error message is displayed.

Chrome OS: 11021.33.0
Chrome: 70.0.3538.41
Device: Santa

Sign in to add a comment