1390372 - Follow-up: set top site title to page title if the uri has a path

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, enhancement, P1)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Part 2 of bug 1382332, which is about distinguishing multiple top sites/highlights from the same domain. To summarize our options:

(In reply to Michael Comella (:mcomella) from comment #25)
> To summarize the consequences of comment 23, I think there are two variables
> we can tweak are:
> 
> 1) Revise the UI to help distinguish multiple sites with same host (e.g.
> allow titles to take up multiple lines only if there is another top site
> with the same host)
> 
> 2) Change the top sites algorithm so we have fewer sites with the same host.
> 
> In my opinion, I think we should focus on #1: afaik, there's been no desire
> to modify our existing top sites query so it's probably pretty good (caveat:
> users used to be able to enter custom sites). The approach of limiting the
> number of same-host sites that appear in our top sites doesn't seem to be a
> better user experience to me (e.g. if I use my browser primarily to look at
> github; and how my top sites on desktop doesn't seem to accurately represent
> my browser usage).

And to summarize the starting approach for this bug:

(In reply to Michael Comella (:mcomella) from bug 1382332 comment #40)
> To follow up on comment 25, spoke with bbell: we're going to try using
> "subdomain.domain" when the page does not have a path and use the page title
> when the page has a path.
> 
> fwiw, he mentioned nchapman has some code that will extract the more
> relevant parts out of a url (e.g. would pull out the Bug # for Bugzilla) but
> we haven't had a chance to talk to him and to me it sounds like a heuristic
> rabbit hole so we'll try ^ first.

Note that screenshots (bug 1389259) are another way to disambiguate but may not work well (depending on the screenshot) and may be more work than we want to invest here.

---

Changes in this bug rely on code from bug bug 1386902.

I blocked bug 1382332 because it's not done without this follow-up bug.

Adding to the current sprint as a follow-up.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot post-patch

Bryan, I think the titles can be better but we don't always have space for it, especially once we put the pin icon in place (e.g. we have 10 characters for Dark Sky, which is one character short of an address indicator).

Any ideas?

We could just land this to see how well it does on people's devices sites, rather than me cherry-picking. Maybe it gets us to 80%?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1390372: Add URIUtils.isPathEmpty.

Review commit: https://reviewboard.mozilla.org/r/168528/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168528/

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1390372: Set top site title to page title if path is non-empty.

Review commit: https://reviewboard.mozilla.org/r/168530/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168530/

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Spoke with bbell: we're going to land this as is and see how it feels - maybe it'll cover the 90% of cases.

Comment 5

[Chenxia Liu [:liuche]]

Comment on attachment 8897244 [details]
Bug 1390372: Add URIUtils.isPathEmpty.

https://reviewboard.mozilla.org/r/168528/#review176966

Comment 6

[Chenxia Liu [:liuche]]

Comment on attachment 8897245 [details]
Bug 1390372: Set top site title to page title if path is non-empty.

https://reviewboard.mozilla.org/r/168530/#review176974

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6de0b3854124
Add URIUtils.isPathEmpty. r=liuche
https://hg.mozilla.org/integration/autoland/rev/f273e82ed541
Set top site title to page title if path is non-empty. r=liuche

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6de0b3854124
https://hg.mozilla.org/mozilla-central/rev/f273e82ed541

Comment 9

[Paul]

Attachment: Some ambiguity after patch

A post-patch screenshot, showing five Top Sites entries belonging to the Bureau of Meteorology

Comment 10

[Paul]

That is an improvement, as I can now distinguish two of the five Top Sites entries from one site (3rd through 7th), see screenshot: https://bug1390372.bmoattachments.org/attachment.cgi?id=8900947

In this state though, Top Sites is still less usable than the old style. Now we need to remove the remaining ambiguity.

I suggest detecting that multiple entries have the same *visible* description (because here the titles are different, but we only see the first word), and in those cases, falling back to screenshots instead of favicons.

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Paul from comment #10)
> I suggest detecting that multiple entries have the same *visible*
> description (because here the titles are different, but we only see the
> first word), and in those cases, falling back to screenshots instead of
> favicons.

In bug 1389259, we were going to try adding screenshots for any page that has a path (e.g. google.com/search but not google.com). However, in the current implementation, we're not always guaranteed to have a screenshot, e.g. if we delete the screenshots due to memory pressure (I think there are more details in the bug).

NI bbell to see Paul's situation (attachment in comment 9) – any new thoughts or do we want to see how it looks after bug 1389259?

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Paul, I'm not sure what's going on with the two first pinned sites ("?" favicon, no title) – do you think that's something that needs to be addressed or perhaps this is something private? If the former, is there already a bug open for it?

Comment 13

[Paul]

Those first two entries point to URLs that generate 301 redirects. This is intentional, and fits in to my workflow: I visit the page, and before it can redirect I edit the URL. Basically it's like a URL template.

Firefox does not handle this situation perfectly:

1. It generates no preview icon or description
2. The entry must be pinned, otherwise it is hidden
3. I suspect the visit counter is not being incremented, although I have no way to check that

At least Firefox honours these entries, so I am satisfied with the way it works. Thanks for asking.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #11)
> NI bbell to see Paul's situation (attachment in comment 9) – any new
> thoughts or do we want to see how it looks after bug 1389259?

Spoke with bbell: screenshots (bug 1389259), which we may get to for 57, may help, but otherwise we should aim for the ultimate goal of allowing users to customize their sites titles rather than trying to make our algorithm perfect for all of the edge cases: see bug 1394534).