We want to be able to do the following: www.mozilla.org -> mozilla foobar.blogspot.com -> foobar m.spiegel.de -> spiegel I think it's probably enough to use stripCommonSubdomains(), then take the first part of the url (split on "."), but maybe there's some utility that would allow us to do this?

What do we need this for? Sounds like something that could be pretty complicated (or impossible) to do right for all edge cases.

(In reply to Sebastian Kaspari (:sebastian) from comment #2) > What do we need this for? Sounds like something that could be pretty > complicated (or impossible) to do right for all edge cases. The mockups seem to use strings like: "ask" for ask.com "slate" for slate.com "arstechnica" for arstechnica.com The full page title doesn't fit in the topsites card (see attached screenshot), using the URL has a similar issue. I guess a compromise would be to strip common prefixes (m./mobile./www.) and then show as much of the URL as possible? I agree that this seems tricky to get right, I wonder if there are alternative solutions to displaying enough descriptive text? Looking at Firefox iOS: they seem to use the full page title in a 3x3 topsites grid, albeit with smaller text: maybe we just need to use a similar or smaller text size for our 4x1 grid?

I'm renaming this bug since there might be alternative solutions. E.g. using the full title spread over 2 lines of text seems to provide sufficient information for me, which might be the best solution and/or an interim solution.

Here's a screenshot with 2 lines of text in the title. This seems to be a touch more legible than sticking to one line, while still using the page title.

That's how the desktop add-on wants to improve the algorithm they are using: https://github.com/mozilla/activity-stream/issues/1311 We should try to follow their lead here, I guess. It will still be hard or near to impossible to make this work for the whole of the web. But we can explore/improve this together with the desktop team going forward.

Comment on attachment 8799717 [details] Bug 1299201 - TestStringUtils: Remove unneeded Assert prefix. https://reviewboard.mozilla.org/r/84852/#review83542

Comment on attachment 8799718 [details] Bug 1299201 - StringUtils: Add helper method for joining strings with a separator. https://reviewboard.mozilla.org/r/84854/#review83546 All hail java8's String.join and StringJoiner, but I guess we're stuck with this. ::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/StringUtils.java:282 (Diff revision 1) > + * Joining together a sequence of strings with a separator. > + */ > + public static String join(@NonNull String separator, @NonNull List<String> parts) { > + final StringBuilder builder = new StringBuilder(); > + > + for (int i = 0; i < parts.size(); i++) { You could unroll the loop a bit to make this a tad faster: ``` if (parts.size() == 0) { return ""; } builder.append(parts.get(0)); for (int i = 1; i < parts.size(); i++) { builder.append(separator); builder.append(parts.get(i)); } ```

Comment on attachment 8799719 [details] Bug 1299201 - Add helper method for stripping the public suffix from a domain. https://reviewboard.mozilla.org/r/84856/#review83560 I've only briefly looked through the trie stuff, I'm guessing that's what you've copied over? ::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/publicsuffix/PublicSuffix.java:36 (Diff revision 1) > + * > + * www.mozilla.org -> www.mozilla > + * independent.co.uk -> independent > + */ > + @Nullable > + public static String stripPublicSuffix(@Nullable String domain) { I'm really not a fan of allowing null as either a parameter or (worse) a return value... I'd rather callers of this not do silly things like try and strip a public suffix off of a null in the first place. Left as is, IDE will now highlight unchecked uses of this function's return value as potential NPEs (and it'll be right to do so), but that seems unnecessary. ::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/publicsuffix/PublicSuffix.java:81 (Diff revision 1) > + > + /** > + * Normalize domain and split into domain parts (www.mozilla.org -> [www, mozilla, org]). > + */ > + private static List<String> normalizeAndSplit(String domain) { > + domain = domain.replace("[.\u3002\uFF0E\uFF61]", "."); // All dot-like characters to '.' I don't think your unit tests cover dot-like character parsing. ::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/publicsuffix/PublicSuffix.java:82 (Diff revision 1) > + /** > + * Normalize domain and split into domain parts (www.mozilla.org -> [www, mozilla, org]). > + */ > + private static List<String> normalizeAndSplit(String domain) { > + domain = domain.replace("[.\u3002\uFF0E\uFF61]", "."); // All dot-like characters to '.' > + domain = domain.toLowerCase(); for clarity i'd put these values into a new string object... Although that won't change anything functionally. ::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/publicsuffix/PublicSuffix.java:103 (Diff revision 1) > + */ > + private static int joinIndex(List<String> parts, int index) { > + int actualIndex = 0; > + > + for (int i = 0; i < index; i++) { > + if (i > 0) { (i guess this is the same comment about unrolling a for loop as I've made on an earlier commit) -> could just start iterating at i=1 and drop this if.

Comment on attachment 8799720 [details] Bug 1299201 - Introduce ActivityStream.extractLabel() to extravt a label from a URL. https://reviewboard.mozilla.org/r/84858/#review83564 Code seems fine. Speaking more broadly, I'm wondering if this effort isn't misguided in the first place. People extract all sorts of information out of URLs, and do so often in complicated and hard-to-predict ways. They extract meaning according to rules that are by-product of person's interaction with a website, and with the internet at large. We're bound to get this wrong, and annoy a lot of people, all in the name of "our UI doesn't have enough space for longer URLs". Perhaps the problem here are not the long URLs. My personal pet peeves that come up right away: - amazon.ca and amazon.com will become amazon. That is simply wrong, and will be very annoying. These are very different websites, as far as I'm concerned. - reddit.com/r/firefox will become firefox. It _should be_ /r/firefox, but our rules don't know about reddit conventions, of course. Hard-coding some one-off rules for different websites might work for, perhaps, top 100, but is bound to miss the mark for majority of the web.

(In reply to :Grisha Kruglov from comment #17) > Speaking more broadly, I'm wondering if this effort isn't misguided in the > first place. People extract all sorts of information out of URLs, and do so > often in complicated and hard-to-predict ways. They extract meaning > according to rules that are by-product of person's interaction with a > website, and with the internet at large. We're bound to get this wrong, and > annoy a lot of people, all in the name of "our UI doesn't have enough space > for longer URLs". Perhaps the problem here are not the long URLs. > > My personal pet peeves that come up right away: > - amazon.ca and amazon.com will become amazon. That is simply wrong, and > will be very annoying. These are very different websites, as far as I'm > concerned. > - reddit.com/r/firefox will become firefox. It _should be_ /r/firefox, but > our rules don't know about reddit conventions, of course. > > Hard-coding some one-off rules for different websites might work for, > perhaps, top 100, but is bound to miss the mark for majority of the web. See comments above and discussion here: https://github.com/mozilla/activity-stream/issues/1311 I absolutely agree that it will be nearly impossible to have a solution that scales to the craziness of the web. The desktop team still wants to try and explore - and for the sake of consistency I implemented this. For other parts of the UI the desktop team is using provider_name from embed.ly - That's something we might be trying to add to our metadata parser too (It seems to be based on og:site_name and similar values). After that the URL based approach might just be a fallback. See https://github.com/mozilla/activity-stream/issues/1473

Pushed by s.kaspari@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b9a4f60323cc TestStringUtils: Remove unneeded Assert prefix. r=Grisha https://hg.mozilla.org/integration/autoland/rev/8b2efd8c1d86 StringUtils: Add helper method for joining strings with a separator. r=Grisha https://hg.mozilla.org/integration/autoland/rev/a9f816396d7e Add helper method for stripping the public suffix from a domain. r=Grisha https://hg.mozilla.org/integration/autoland/rev/f4d4225c3c0e Introduce ActivityStream.extractLabel() to extravt a label from a URL. r=Grisha

https://hg.mozilla.org/mozilla-central/rev/b9a4f60323cc https://hg.mozilla.org/mozilla-central/rev/8b2efd8c1d86 https://hg.mozilla.org/mozilla-central/rev/a9f816396d7e https://hg.mozilla.org/mozilla-central/rev/f4d4225c3c0e