Closed Bug 1116563 Opened 9 years ago Closed 9 years ago

Reading list items added from share overlay have no content in reading list

Categories

(Firefox for Android Graveyard :: Awesomescreen, defect)

All
Android
defect
Not set
normal

Tracking

(firefox36 verified, firefox37 verified, fennec36+)

VERIFIED FIXED
Firefox 37
Tracking Status
firefox36 --- verified
firefox37 --- verified
fennec 36+ ---

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached image 2014-12-30 20.00.32.png
See screenshot. This is fallout from bug 889351. We should probably fall back to showing a URL if we don't have a title or description.
Is this for every item? This should work in some cases, e.g., sharing from a Firefox:

        ContentValues values = new ContentValues();
        values.put(Bookmarks.TITLE, shareData.title);
        values.put(Bookmarks.URL, shareData.url);

        browserDB.addReadingListItem(resolver, values);
(In reply to Richard Newman [:rnewman] from comment #1)
> Is this for every item? This should work in some cases, e.g., sharing from a
> Firefox:
> 
>         ContentValues values = new ContentValues();
>         values.put(Bookmarks.TITLE, shareData.title);
>         values.put(Bookmarks.URL, shareData.url);
> 
>         browserDB.addReadingListItem(resolver, values);

Confirmed, I see we get a title when sharing from another Firefox instance.
Assignee: nobody → margaret.leibovic
antlam, what kind of fallback would you like to see if a reading list item doesn't have a title and/or excerpt. At the very least I think we should always display something in the title TextView, so if there is no title we should show the URL. Do you think we should also fall back to showing a URL in the excerpt TextView?

In the future we will hopefully be better about ensuring we have title/excerpt data for every reading list item, but in the meantime we should get some fallbacks in place.
Flags: needinfo?(alam)
Attachment #8543428 - Flags: review?(rnewman)
/r/1983 - Bug 1116563 - Fall back to display URL if reading list item has no title

Pull down this commit:

hg pull review -r 60c6805d2b3efd9794f9b3791b68232a477c6bdc
Ah, thanks for filing this! 

(In reply to :Margaret Leibovic from comment #3)
> antlam, what kind of fallback would you like to see if a reading list item
> doesn't have a title and/or excerpt. At the very least I think we should
> always display something in the title TextView, so if there is no title we
> should show the URL. 

Good idea. I think that makes sense. But we should probably not show repetitive info like http:// or WWW.

> Do you think we should also fall back to showing a URL
> in the excerpt TextView?

Yeah, that sounds like a good start. But I'm imagining double URL's and it's kinda weird. Not sure if it will come up but could we do these as fall backs?

Title, no excerpt: 
 - Excerpt is full URL

No Title, no excerpt: 
 - Title is compact URL
 - Excerpt is full URL 

No Title, excerpt (does this even exist?):
 - Title is compact URL


> In the future we will hopefully be better about ensuring we have
> title/excerpt data for every reading list item, but in the meantime we
> should get some fallbacks in place.

Yes. Let's not have this be a long term solution :)
Flags: needinfo?(alam) → needinfo?(margaret.leibovic)
Comment on attachment 8543428 [details]
MozReview Request: bz://1116563/margaret

Clearing review until I address antlam's comments.
Flags: needinfo?(margaret.leibovic)
Attachment #8543428 - Flags: review?(rnewman)
(In reply to Anthony Lam (:antlam) from comment #6)
> Ah, thanks for filing this! 
> 
> (In reply to :Margaret Leibovic from comment #3)
> > antlam, what kind of fallback would you like to see if a reading list item
> > doesn't have a title and/or excerpt. At the very least I think we should
> > always display something in the title TextView, so if there is no title we
> > should show the URL. 
> 
> Good idea. I think that makes sense. But we should probably not show
> repetitive info like http:// or WWW.
> 
> > Do you think we should also fall back to showing a URL
> > in the excerpt TextView?
> 
> Yeah, that sounds like a good start. But I'm imagining double URL's and it's
> kinda weird. Not sure if it will come up but could we do these as fall backs?
> 
> Title, no excerpt: 
>  - Excerpt is full URL
> 
> No Title, no excerpt: 
>  - Title is compact URL
>  - Excerpt is full URL 
> 
> No Title, excerpt (does this even exist?):
>  - Title is compact URL

I doubt this would exist, but it would be handled by the case that we would just show a compact URL for the title regardless of whether or not there is an excerpt. I think this logic could also be expressed as:

No title? Title is compact URL.
No excerpt? Excerpt is full URL.
Attachment #8543428 - Flags: review?(rnewman)
/r/1983 - Bug 1116563 - Fall back to display URL if reading list item has no title. r=rnewman

Pull down this commit:

hg pull review -r bd72a15c6f7745c86d3ac01e0b3689c6d35d745e
(In reply to :Margaret Leibovic from comment #8)

> No title? Title is compact URL.
> No excerpt? Excerpt is full URL.

Maybe if there's no content yet, show the URL in the title, and "Not yet downloaded" as the excerpt?

Trying to capture the why/what/what next for the user.
Status: NEW → ASSIGNED
Version: Firefox 35 → Trunk
Attachment #8543428 - Flags: review?(rnewman) → review+
(In reply to Richard Newman [:rnewman] from comment #10)
> (In reply to :Margaret Leibovic from comment #8)
> 
> > No title? Title is compact URL.
> > No excerpt? Excerpt is full URL.
> 
> Maybe if there's no content yet, show the URL in the title, and "Not yet
> downloaded" as the excerpt?
> 
> Trying to capture the why/what/what next for the user.

I think we can look at updating this string when we start to actually download and update the content (bug 1113454), since we don't actually ever download the content to update that excerpt right now :)

https://hg.mozilla.org/integration/fx-team/rev/2eee3736551a
https://hg.mozilla.org/mozilla-central/rev/2eee3736551a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Bug 889351 is on Fx36. Need uplift?
tracking-fennec: ? → 36+
Comment on attachment 8543428 [details]
MozReview Request: bz://1116563/margaret

Approval Request Comment
[Feature/regressing bug #]: bug 889351
[User impact if declined]: reading list items may have no content
[Describe test coverage new/current, TBPL]: no test coverage, tested locally and baked on m-c
[Risks and why]: low-risk, small tweak to what we display in reading list
[String/UUID change made/needed]: none
Attachment #8543428 - Flags: approval-mozilla-aurora?
Attachment #8543428 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in builds:
- 36 Beta 1;
- 37.0a1 (2015-01-12).
Device: Samsung Galaxy S4 (Android 4.4.2).
Status: RESOLVED → VERIFIED
Attachment #8543428 - Attachment is obsolete: true
Attachment #8618998 - Flags: review+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.