To avoid having a ReadingList that is initially empty and doesn't look terribly useful, we want to pre-populate it with a small set of pages.

Guess this will be part of the initial database creation routine, getting the data from a mix of static data (url, image) and l10n strings (title).

Markus/Holly: How close is this to being defined? Because this requires localised strings in-product, we have the l10n deadline. Don't need the content for the pages, just: * URL (can change later) * Image (can change later) * Title (can't change after l10n deadline)

Holly: I see you already have titles for the 3 SUMO Articles in your design (http://cl.ly/image/2h0j3J0w1S16). We can run them by Matej and are done with that. I do not think we necessarily need to have the Title correlate with the Title of the SUMO article. Otherwise we need to add Joni to this discussion, as she is currently writing the SUMO articles. Do you have a suggestion for the Moz.org page title? How about: Better Reading, Better Organized.

(In reply to Markus Jaritz [:maritz] from comment #3) > Holly: I see you already have titles for the 3 SUMO Articles in your design > (http://cl.ly/image/2h0j3J0w1S16). We can run them by Matej and are done > with that. I do not think we necessarily need to have the Title correlate > with the Title of the SUMO article. Otherwise we need to add Joni to this > discussion, as she is currently writing the SUMO articles. > Do you have a suggestion for the Moz.org page title? How about: Better > Reading, Better Organized. Hi Markus, Those SUMO titles are directly from your wireframes. If you'd like to change them at all before running by Matej, let me know. Web Prod will also provide a title for the Mozilla.org page that populates the list. I like your suggestion, Markus. I'll run it by Matej. Blair, We're aware of the March 15th string freeze, but since we are meeting about copy next week with Matej, we should get strings to you before that date.

(In reply to Blair McBride [:Unfocused] (I don't read bugmail - needinfo? me!) from comment #2) > Markus/Holly: How close is this to being defined? Because this requires > localised strings in-product, we have the l10n deadline. > > Don't need the content for the pages, just: > * URL (can change later) > * Image (can change later) > * Title (can't change after l10n deadline) URL for Mozilla.org page populating the readign list https://bugzilla.mozilla.org/show_bug.cgi?id=1134286#c4 Title for that page and SUMO article titles and URLs coming soon.

See bug 1138738 for all Reading List onboarding copy. https://bugzilla.mozilla.org/show_bug.cgi?id=1138738#c4 4 article titles (3 SUMO, 1 Mozilla.org page) and URLs are also provided. Keep in mind that locales (e.g., /en-us/) should not be in the URLs. The existing Sync article title will need to be overridden when populated in the Reading List. Markus mentioned this is possible. Blair, can you confirm? Markus, can you make sure SUMO has the new article titles for RV and RL articles? (they should not change the Sync article title on their end)

(In reply to Holly Habstritt Gaal [:Habber] from comment #6) > The existing Sync article title will need to be overridden when populated in > the Reading List. Markus mentioned this is possible. Blair, can you confirm? Correct.

Pre-landing strings. Nothing terribly fancy here. Expecting the URLs to be stored in prefs, which can be done at any time.

I find these strings hard to translate without at least an outline of the correspondig web pages. (And yes, don't put locale codes into the url, we should rely on server-side negotiation) CCing Elan, as I don't think we're having SUMO articles on our release tracking yet?

(In reply to Axel Hecht [:Pike] from comment #9) > I find these strings hard to translate without at least an outline of the > correspondig web pages. > > (And yes, don't put locale codes into the url, we should rely on server-side > negotiation) > > CCing Elan, as I don't think we're having SUMO articles on our release > tracking yet? Hi Axel, URLs are in the corresponding copy doc here (scroll all the way to bottom for these articles) https://bug1131112.bugzilla.mozilla.org/attachment.cgi?id=8564217

The second SUMO one already redirects? The SUMO articles exist already, that's good. Can we add the links to the localization notes? For the mozilla.org page, maybe link to the dev server from the localization notes? It doesn't have the page yet, but it's going to be the first to have it, and have it localized.

Comment on attachment 8574606 [details] [diff] [review] Strings v1 Review of attachment 8574606 [details] [diff] [review]: ----------------------------------------------------------------- I'm confused why we need to preland - there's still quite a few weeks of 39 left, right? Axel's comment makes me more worried still. :-(

Comment on attachment 8574606 [details] [diff] [review] Strings v1 Review of attachment 8574606 [details] [diff] [review]: ----------------------------------------------------------------- I love missing comments. Anyway, what Axel said, but also: ::: browser/locales/en-US/chrome/browser/browser.properties @@ +715,5 @@ > readingList.promo.firstUse.syncSignedIn.body = Open your Reading List articles everywhere you use $S. > readingList.promo.firstUse.syncSignedIn.moveToButton = Next: Easy access » > + > +# Pre-landed strings for bug 1136570 > +# LOCALIZATION NOTE(readingList.prepopulatedArticles.learnMore): $S is brandShortName While money makes the world go round, you want "%" not "$" here. @@ +718,5 @@ > +# Pre-landed strings for bug 1136570 > +# LOCALIZATION NOTE(readingList.prepopulatedArticles.learnMore): $S is brandShortName > +readingList.prepopulatedArticles.learnMore = Learn how %S makes reading more pleasant > +readingList.prepopulatedArticles.supportReadingList = Save, sync and read pages anywhere with Reading List > +readingList.prepopulatedArticles.supportReaderView = Enjoy clutter-free Web pages with Reader View This should probably have a note about "Reading List" and "Reading View" being new parts of Firefox, similar to what we have on Android, and therefore being proper names which have capitals in most languages (at least not just German) ? :-) @@ +719,5 @@ > +# LOCALIZATION NOTE(readingList.prepopulatedArticles.learnMore): $S is brandShortName > +readingList.prepopulatedArticles.learnMore = Learn how %S makes reading more pleasant > +readingList.prepopulatedArticles.supportReadingList = Save, sync and read pages anywhere with Reading List > +readingList.prepopulatedArticles.supportReaderView = Enjoy clutter-free Web pages with Reader View > +# LOCALIZATION NOTE(readingList.prepopulatedArticles.learnMore): $S is syncBrandShortName Some more money that crept in.

(In reply to Axel Hecht [:Pike] from comment #11) > Can we add the links Yep, can do - good idea. (In reply to :Gijs Kruitbosch from comment #12) > I'm confused why we need to preland - there's still quite a few weeks of 39 > left, right? Because this is needed for 38 :( Thankfully we have a l10n extension period for the ReadingList project and the crazy ship date we're aiming for. (In reply to :Gijs Kruitbosch from comment #13) > While money makes the world go round, you want "%" not "$" here. *facepalm* This is why I like our code review process. > This should probably have a note about "Reading List" and "Reading View" > being new parts of Firefox, similar to what we have on Android, and > therefore being proper names which have capitals in most languages (at least > not just German) ? :-) Another fine idea!

Comment on attachment 8575203 [details] [diff] [review] Patch v1.1 Review of attachment 8575203 [details] [diff] [review]: ----------------------------------------------------------------- I am sad that apparently we are not oxford comma people. In any case, r=me ;-)

SUMO has the new article titles, and did change them. This however changed the respective URLs: https://support.mozilla.org/kb/save-sync-and-read-pages-anywhere-reading-list https://support.mozilla.org/kb/enjoy-clutter-free-web-pages-reader-view

(In reply to :Gijs Kruitbosch from comment #16) > I am sad that apparently we are not oxford comma people. I'm having an existential crisis over this myself. (In reply to Markus Jaritz [:maritz] from comment #17) > This however changed the respective URLs: Oh, thanks - have updated the comments to reflect that. https://hg.mozilla.org/integration/fx-team/rev/a84f2773ca6f

Pre-landing even more strings for ReadingList.

Comment on attachment 8575811 [details] [diff] [review] Patch v1.2 Approving for uplift to aurora since we want to have this as part of the Reading List feature.

I'm not seeing any pages pre-populated into the Reading List as of Nightly 39.0a1 (2015-03-15). Is there a pref I should be using to enable this feature?

No, this hasn't bee implemented. We just prelanded strings here.

For our FTU-tour to introduce the feature as planed Bug 1134446 we need this content in the sidebar. And we also need the sidebar to be open for that (Bug 1134446) Can we get both these things done for 38?

dropping this back to the default assignee so someone else can pick it up.

(In reply to Markus Jaritz [:maritz] from comment #24) > For our FTU-tour to introduce the feature as planed Bug 1134446 we need this > content in the sidebar. And we also need the sidebar to be open for that > (Bug 1134446) Can we get both these things done for 38? This is marked as a P1, meaning it's a hard requirement for shipping. The other bug is only P3 (because the feature still works/feels like it works without it), but it does have a patch.

(In reply to Blair McBride [:Unfocused] (I don't read bugmail - needinfo? me!) from comment #26) > (In reply to Markus Jaritz [:maritz] from comment #24) > > For our FTU-tour to introduce the feature as planed Bug 1134446 we need this > > content in the sidebar. And we also need the sidebar to be open for that > > (Bug 1134446) Can we get both these things done for 38? > > This is marked as a P1, meaning it's a hard requirement for shipping. The > other bug is only P3 (because the feature still works/feels like it works > without it), but it does have a patch. The feature does work, but the FTU Holly designed assumes that the content in the sidebar is visible. So if it is not, the tour we created will not be as effective as intended. I guess that someone is working on opening the sidebar is a good sign.

Comment on attachment 8575811 [details] [diff] [review] Patch v1.2 Ugh, so, apparently landing the strings-only patch on Aurora got overlooked (I've been off sick for some time). So re-requesting Aurora approval. This is *really* late in the game for l10n, but we've been considering this bug as a hard blocker for shipping ReadingList, and we figured a late string change would likely come up at some stage.

(In reply to Blair McBride [:Unfocused] (I don't read bugmail - needinfo? me!) from comment #28) > we figured a late string change would likely come up at some stage. Just noting that this was expected in the first 2/3 weeks of the cycle, not at the end of the 5th.

Comment on attachment 8575811 [details] [diff] [review] Patch v1.2 Seems that flod does not veto this late uplift, a+

Flod didn't really comment on yes or not, nor will I without actually looking at this bug.

Comment on attachment 8575811 [details] [diff] [review] Patch v1.2 Setting this back to '?' for aurora. I think we should triage this aspect of the reading list feature in the light of it missing the extended string freeze deadline, and the current state of development of reader view/reading list. This might just as well be a good time to let some pressure out of the scope of this feature for 38. Flagging dolske for triage, we can also sync up on this in the coordination meeting later today.

We need these strings in 38.

WiP of implementation. Not going any further on this until the above comments are resolved. Issues: * Will re-add new items when creating a new profile and linking with Sync (bookmarks has this issue too, IIRC) * No preview images * Because of server-side redirects, these items won't be shown as selected * URLs need the tracking params added Having it detect when one of these items is loaded in the current tab, with the server-redirected URL, is a bit of a pain. I thought maybe we could use canonical URL metadata, but that doesn't help in these cases (though we'll probably want that in v2 anyway). Easiest solution I can think of so far is to include %LOCALE% in the URLs, and avoid server-side redirects. Alternative solution, which is more engineering effort, is to add an API (and local/server-side DB fields) to allow matching on wildcards.

(In reply to Blair McBride [:Unfocused] (I don't read bugmail - needinfo? me!) from comment #34) > Alternative solution, which is more engineering effort, is to add an API > (and local/server-side DB fields) to allow matching on wildcards. Hm, could cheat and hard-code exceptions for just these pages, or just the mozilla.org domain.

Taking gavin's word here that we have to, clearing needinfo on dolske. Sylvestre, can you flag the approval again? Thanks.

Comment on attachment 8575811 [details] [diff] [review] Patch v1.2 Thanks for the productive chat :)

(In reply to Drew Willcoxon :adw from bug 1124153 comment #14) > FYI Blair, bug 1147554 is fixing a Talos regression in startup IO by lazily > creating the DB connection. It's not 100% clear yet if that fix will work, > since even if ReadingList internally lazily creates its connection, it's for > naught if some consumer forces it to create its connection on startup. But > assuming it does work, you'll probably want to move your _prepopulate call > from the ctor to some new method that the store would call when it creates > its connection. Yea, I had been thinking having that in the ctor wasn't ideal. Another alternative I'd been considering is have the UI specify when it needs it (ie, showing sidebar, showing bookmarks menu) - which could potentially reduce cases when we re-add these items in a new profile before existing ReadingList data is synced. That might be a little too magical, though.

Needing some time off, so attaching my latest WiP for someone else to maybe pickup. Issues outstanding include those in comment 34 and comment 39.

Justin, do you want to fix this issue in 38? If yes, who can work on this? Thanks

Given bug 1155515, I don't think this bug is the most urgent to work on for me now, but I had already unbirotted this, so I'm attaching the unbitrotted version.