1009574 - Properly handle redirects on suggested sites

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Aaron Train [:aaronmt]]

See screenshot.

I think if we do suggested site defaults we should try not to breadcrumb the other top-site spots on access, it looks sloppy that we create entries for the same sites.

Comment 1

[Aaron Train [:aaronmt]]

Attachment: screenshot.png

Comment 2

[Lucas Rocha (:lucasr)]

The current suggested sites redirect to different URLs causing the history entries to have different URLs. We need to add a way for suggested sites to account for that somehow. Maybe a pre-defined pattern or something?

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Can we just used the final URL in the suggested site JSON?

Comment 4

[Lucas Rocha (:lucasr)]

(In reply to Mark Finkle (:mfinkle) from comment #3)
> Can we just used the final URL in the suggested site JSON?

For default sites, yes, I could just use the final en-US URL directly in the JSON files. Those URLs can then be overridden by each locale. But I wonder if that's future-proof enough though... Maybe I'm just over-engineering the solution here :-)

Comment 5

[Lucas Rocha (:lucasr)]

Attachment: Use final en-US specific URLs on suggested sites (r=mfinkle)

Comment 6

[Lucas Rocha (:lucasr)]

Comment on attachment 8421749 [details] [diff] [review]
Use final en-US specific URLs on suggested sites (r=mfinkle)

Ok, let's do the simplest possible thing for now. Note to self: do the same on the final list of suggested sites in bug 997765.

Comment 7

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/integration/fx-team/rev/ac81613f2c0e

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ac81613f2c0e

Comment 9

[Francesco Lodolo [:flod]]

A couple of questions: 
* why http and not https for mozilla.org?
* why adding en-US to the URL? This prevents us to reuse this in l10n (I confess, this entire tile thing is a gigantic question mark on my side).

Comment 10

[Lucas Rocha (:lucasr)]

(In reply to Francesco Lodolo [:flod] from comment #9)
> A couple of questions: 
> * why http and not https for mozilla.org?

My bad, I should have used https there. However, keep in mind that these websites are just placeholders while we come up with the actual list of suggested sites in bug 997765. These changes here are just to avoid issues with the temporary list.

> * why adding en-US to the URL? This prevents us to reuse this in l10n (I
> confess, this entire tile thing is a gigantic question mark on my side).

The build system for suggested sites is very similar to the one for search plugins. mobile/android/base/locales/en-US/suggestedsites/list.txt defines the list of suggested sites, one site name per line. For each site name, there should be a matching sitename.json file in the same directory defining title, URL, and background color.

All these files (list.txt and json files for each site) can be overridden in each l10n repo under mobile/android/base/suggestedsites/. Most of the time, locales will be overriding the json files with translations for the title and possibly defining the locale-specific URL for the site.

I hope that clarifies things a bit.

Comment 11

[Axel Hecht [:Pike]]

I think the way this is set up isn't working out great. Search setup is hacky, and this is slightly different enough for us to need fixes.

I don't think the json in the l10n files make sense, as they're useless without the images that they reference. I'd rather have them all in base, probably even outside of /en-US/. Having list.txt in the localized files is OK, but I'd only start putting them there once we actually know what we want to put there, and how to choose the right content.

Doing it now, and then doing it again some 40 times is just hard.

PS: You also missed the searchengine part in filter.py.

Comment 12

[Lucas Rocha (:lucasr)]

(In reply to Axel Hecht [:Pike] from comment #11)
> I think the way this is set up isn't working out great. Search setup is
> hacky, and this is slightly different enough for us to need fixes.
> 
> I don't think the json in the l10n files make sense, as they're useless
> without the images that they reference. I'd rather have them all in base,
> probably even outside of /en-US/. Having list.txt in the localized files is
> OK, but I'd only start putting them there once we actually know what we want
> to put there, and how to choose the right content.

Not sure I follow. We need to talk :-) Let's map the parts of this setup that are likely to cause pain on the l10n side and fix those. We still have plenty of time.

> Doing it now, and then doing it again some 40 times is just hard.
> 
> PS: You also missed the searchengine part in filter.py.

searchengine part? filter.py?

Comment 13

[Francesco Lodolo [:flod]]

(In reply to Lucas Rocha (:lucasr) from comment #12)
> searchengine part? filter.py?

For reference
http://hg.mozilla.org/mozilla-central/file/default/browser/locales/filter.py

That means that the system is currently complaining for missing .json files in l10n repos, while it shouldn't (like it does for .xml files).

Comment 14

[Lucas Rocha (:lucasr)]

FYI: I filed bug 1011620 to revisit how suggestedsite.json is generated to be more l10n-friendly.