Synced tabs from iOS don't include title, and might include localhost URLs as the title

RESOLVED FIXED in 3.0

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: rfeeley, Assigned: fluffyemily)

Tracking

unspecified
All
iOS

Firefox Tracking Flags

(fxios-v3.0 fixed, fxios+)

Details

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
Created attachment 8721544 [details]
synced-tabs.png

Synced tabs from iOS only showing URL in desktop, not the page title. Not sure if this is an iOS bug, a desktop bug or a toolkit bug?
I'm guessing all your pages don't have titles. :)

Most of my tabs for iOS have titles.
I can repro this.

JSON.stringify(Weave.Service.engineManager.get("tabs")._store._remoteClients)
=>

"{"ekhsSAMMigaU":{"id":"ekhsSAMMigaU","tabs":[{"title":"http://www.buffalosystemsusa.com/mobile/Product.aspx?ProductCode=belay-jacket-limited","icon":null,"urlHistory":["http://www.buffalosystemsusa.com/mobile/Product.aspx?ProductCode=belay-jacket-limited"],"lastUsed":"1455910687846"}…
Hardware: Other → All
Everything storage on down is right, so finger points at this:

    var displayTitle: String {
        if let title = webView?.title {
            if !title.isEmpty {
                return title
            }
        }
        return displayURL?.absoluteString ?? lastTitle ?? ""
    }

… in other words, we're not able to get a title from the webview at the point we write out the tabs, so we're writing the URL instead.

This'll need a little more analysis to find out what's happening when.
tracking-fxios: --- → ?
Rank: 3
tracking-fxios: ? → +
Created attachment 8725722 [details]
remote-tabs.png

Currently none of my remote tabs are shown – only panels
This can't ship as broken.
tracking-fxios: + → ?
Looks like you're seeing something else, Aaron: you're seeing the same home panel URL, presumably because we flushed to the database before restoring tabs. Likely the same root cause, but not the same symptom.

Dumping this in 'General' 'cos this is almost certainly a tabs persistence issue.
Component: Sync → General
Summary: Synced tabs from iOS only showing URL in desktop → Synced tabs from iOS don't include title, and might include localhost URLs as the title
Emily will look at this: related test failure.
Assignee: nobody → etoop
Status: NEW → ASSIGNED
Rank: 3 → 1
tracking-fxios: ? → +
Duplicate of this bug: 1252894
(Assignee)

Comment 9

3 years ago
Created attachment 8728465 [details] [review]
Pull request
Attachment #8728465 - Flags: review?(sleroux)
Attachment #8728465 - Flags: review?(rnewman)
Comment on attachment 8728465 [details] [review]
Pull request

Follow-on/more to think about: are we persisting at the right times?
Attachment #8728465 - Flags: review?(rnewman) → review+
(Assignee)

Comment 11

3 years ago
(In reply to Richard Newman [:rnewman] from comment #10)
> Comment on attachment 8728465 [details] [review]
> Pull request
> 
> Follow-on/more to think about: are we persisting at the right times?

That's definitely something to consider. Right now we are persisting on restore, every time we complete a URL load in a webView and every time we create a new tab. 

We don't really need to persist on restore - we are restoring the state as it was when we last closed the app, so the current DB and disk state on before tab restoration will be identical that what it is after.

We don't need to persist to RemoteTabs when we create a new, blank tab. All this does is replace the existing DB state with ... exactly the same DB state as until we have something loaded in that new tab remote tabs just don't care.

Every time we complete a URL load, sure. This is sensible. This is when we should be persisting.

There may be other times we persist, but nothing that I can see right now.
Assuming that we don't use this particular pile of persisted tab state for anything but Sync, we really just want to persist at two times:

* Immediately before a sync while the app is in the foreground.
* Immediately before we're backgrounded (because if we do it any later, the webviews are gone!).

There's a second decision, which is when/whether to trigger a sync as a result of browsing activity -- browse enough and we should probably sync your tabs… and in the course of so doing we will persist them.

Does that sound sane to you, Emily?
See Also: → bug 1255880
Attachment #8728465 - Flags: review?(sleroux) → review+
(Assignee)

Comment 13

3 years ago
#merged c0b1939721a0ed563fb6582714738b9ddd945e1d
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [needsuplift]
Target Milestone: --- → 3.0
v3.x 2df73de6d3e2a43a53c12f8ac9346bc4c141329a
status-fxios-v3.0: --- → fixed
Whiteboard: [needsuplift]
You need to log in before you can comment on or make changes to this bug.