1251691 - Don't implicitly unwrap NSURLs read from user bookmarks

Status: RESOLVED FIXED

(Firefox for iOS :: Home screen, defect)

Description

[Richard Newman [:rnewman]]

I have a record. It appears as a BookmarkItem in the node tree.

The URL is "http://www.amazon.com/s/ref=nb_sb_noss?url=search-alias%3Ddigital-music&field-keywords=%s"

The data is:

id          guid          type        is_deleted  parentid      parentName      feedUri     siteUri     pos         title       description  bmkUri      tags        keyword     folderName  queryId     server_modified  hasDupe
----------  ------------  ----------  ----------  ------------  --------------  ----------  ----------  ----------  ----------  -----------  ----------  ----------  ----------  ----------  ----------  ---------------  ----------
4932        MUr6CtisszP0  1           0           6Kxq2EXxLzDa  Searches                                        Amazon MP3  Shop over 20 million songs and play your music on your Kindle Fire, Android device, PC, Mac, or iPad with Amazon Cloud Drive and Amazon Cloud Player.  http://www.amazon.com/s/ref=nb_sb_noss?url=search-alias%3Ddigital-music&field-keywords=%s  []          mp3                                 1456279946010    0


BookmarksPanel does this:

        case let item as BookmarkItem:
            homePanelDelegate?.homePanel(self, didSelectURL: NSURL(string: item.url)!, visitType: VisitType.Bookmark)
            break



so we fail to unwrap the URL, because it doesn't parse, because it has a substitution. We should be safer here and elsewhere.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Pull req.

Stacking pull requests as high as they'll go. These are the three commits now on the end -- check the bug numbers.

The behavior now is that a bookmark with a malformed URL will result in _searching_ for that string on the user's default search engine. DuckDuckGo and Google in particular do a good job of this. This is exactly what happens if you copy and paste into the URL bar.

The alternatives are:

* Do nothing -- your tap is wasted.
* Show some kind of toast.
* Put a lot more work into URLFixer (but there are likely to be edge cases always).

ui-review to Robin to make sure she's happy with this experience. (Sure beats a crash, at least.)

Comment 2

[Stephan Leroux [:sleroux]]

Comment on attachment 8724306 [details] [review]
Pull req.

LGTM!

Comment 3

[Richard Newman [:rnewman]]

721a963