Closed Bug 1390372 Opened 2 years ago Closed 2 years ago

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

Categories

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

All
Android
enhancement

Tracking

()

RESOLVED FIXED
Firefox 57
Iteration:
1.28
Tracking Status
firefox57 --- fixed

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

(Whiteboard: [mobileAS] 1.28)

Attachments

(4 files)

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.
Attached image 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%?
Flags: needinfo?(bbell)
Spoke with bbell: we're going to land this as is and see how it feels - maybe it'll cover the 90% of cases.
Flags: needinfo?(bbell)
Attachment #8897244 - Flags: review?(liuche)
Attachment #8897245 - Flags: review?(liuche)
Comment on attachment 8897244 [details]
Bug 1390372: Add URIUtils.isPathEmpty.

https://reviewboard.mozilla.org/r/168528/#review176966
Attachment #8897244 - Flags: review?(liuche) → review+
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
Attachment #8897245 - Flags: review?(liuche) → review+
Pushed by michael.l.comella@gmail.com:
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
https://hg.mozilla.org/mozilla-central/rev/6de0b3854124
https://hg.mozilla.org/mozilla-central/rev/f273e82ed541
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
A post-patch screenshot, showing five Top Sites entries belonging to the Bureau of Meteorology
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.
Attachment #8900947 - Attachment description: bom.png → Some ambiguity after patch
(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?
Flags: needinfo?(bbell)
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?
Flags: needinfo?(mozilla.org)
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.
Flags: needinfo?(mozilla.org)
Summary: Follow-up: it is hard to distinguish multiple top sites/highlights from the same domain → Follow-up: set top site title to page title if the uri has a path
(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).
Flags: needinfo?(bbell)
You need to log in before you can comment on or make changes to this bug.