1195133 - Domain extraction fails for file:/// URIs.

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Richard Newman [:rnewman]]

[Error] [HistorySynchronizer.swift:111] applyRecord: Record application failed: Could not get a domain for file:///var/folders/2b/aaaaaaaaaaaaaaaaaaaaaaa
80000gn/T/aaaaNvZbfz.html#


After three of these fail, we'll abort the sync.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Option 1: don't fail when a URL doesn't have a domain.

Comment 2

[Richard Newman [:rnewman]]

Attachment: Option 2: allow URLs with no domain.

East-coasters for review.

Comment 3

[Stephan Leroux [:sleroux]]

The code for both look good. What does desktop do as far as keeping track of URIs with file://? Not storing them feels cleaner from the code side but I feel like as a user I'd want to see file URIs in my history. Would we have to rework the query for calculating frecency to ignore file:// urls?

Comment 4

[Richard Newman [:rnewman]]

Desktop stores file: URIs. (That's how downloads are stored, too, IIRC.)

The top sites code already transparently drops history items that don't join against the domains table, I think.

The confusion is that file: URIs that sync from Desktop aren't much use on your phone — the purpose of syncing them is for completeness, and to make stuff like file:///Users/sleroux/Dropbox/... work, but that falls down here.

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8648516 [details] [review]
Option 2: allow URLs with no domain.

I like the second option because it is simpler.

Comment 6

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8648515 [details] [review]
Option 1: don't fail when a URL doesn't have a domain.

This looks good but I prefer options #2 because it is simpler.

Comment 7

[Stephan Leroux [:sleroux]]

Comment on attachment 8648516 [details] [review]
Option 2: allow URLs with no domain.

Option 2 as well

Comment 8

[Richard Newman [:rnewman]]

Needs resolvin'.

Comment 9

[Richard Newman [:rnewman]]

We cherry-picked part 1 of option 1:

https://github.com/mozilla/firefox-ios/commit/b55570b318854b0df63af13e4879291eeedb7536

and we'll leave part 2 until we need it.