Context shows the opengraph title instead of the document title, sometimes leading to shortened descriptions of urls

RESOLVED FIXED in Firefox 42

Status

Hello (Loop)
Client
P1
normal
Rank:
12
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: bogdan_maris, Assigned: standard8)

Tracking

unspecified
mozilla43
Points:
2
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox40 wontfix, firefox41+ wontfix, firefox42+ fixed, firefox43 fixed)

Details

(Whiteboard: [context][uxtriage])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8621544 [details]
Screenshot showing the issue

Affected builds:
- Latest Nightly
- Latest Aurora

Affected OS`s:
- Windows 7 64-bit
- Mac OS X 10.9.5
- Ubuntu 14.04 32-bit

STR:
1. Start Firefox
2. Visit http://pocketnow.com/
3. Click hello icon

Expected results: Complete name is shown in hello panel.

Actual results: Incomplete name is shown in hello panel.

'- Smartphone, Tablet, and Wearable Reviews, Videos, Editorials and News'
instead of:
'Pocketnow | Smartphone, Tablet, and Wearable Reviews, Video, Editorial, and News'

Notes:
- Screenshot showing the isssue attached.
- Not a regression, reproducible on Nightly 2015-05-10 where context was enabled by default.
(Reporter)

Comment 1

3 years ago
Apparently not only website titles that contain vertical bar will not be completely displayed, another example:
STR:
1. Visit: https://www.youtube.com/live
2. Click Hello icon

Only '#Live' will appear instead of '#Live - YouTube'
Summary: Incomplete name shown in context if website name contains vertical bar → Incomplete name shown in context if website name contains different characters
(Reporter)

Updated

3 years ago
status-firefox40: --- → affected
status-firefox41: --- → affected
status-firefox42: --- → affected
(Assignee)

Comment 2

3 years ago
I've just dug into this. Looks like its a comes from our "PageMetadata".

In the pages referenced in comment 0 and comment 1, there's two html elements in each that are relevant:

<title>Pocketnow | Smartphone, Tablet, and Wearable Reviews, Video, Editorial, and News</title>
<meta property="og:title" content="- Smartphone, Tablet, and Wearable Reviews, Videos, Editorials and News" />

and:

<title>  #Live - YouTube</title>
<meta property="og:title" content="#Live">

For some reason, probably the open graph stuff, the PageMetadata module is selecting og:title over document.title.

Shane, I believe you know something about this area of the code, do you know why we'd prefer one over the other, should we add in an extra result for Hello which would return the real document.title in addition to the og:title?
Flags: needinfo?(mixedpuppy)
(Assignee)

Comment 3

3 years ago
Also, to clarify the bug, this will only happen on pages that are specifying their open graph document title to be different to that of the page's document title.
Summary: Incomplete name shown in context if website name contains different characters → Context shows the opengraph title instead of the document title, sometimes leading to shortened descriptions of urls

Comment 4

3 years ago
Let's track for FF42 and FF41.
tracking-firefox41: --- → +
tracking-firefox42: --- → +
Probably should remove any overwrites that og: data does, which happens here:

https://mxr.mozilla.org/mozilla-central/source/toolkit/modules/PageMetadata.jsm#182

This doesn't however prevent the opposite, a site that enhances titles in og data but not in title.  The web is messy.
Flags: needinfo?(mixedpuppy)

Comment 6

3 years ago
Mark, here is another one. I do not consider this release blocking and would like to wontfix for FF41 and untrack. We can track it for 42 and consider fixing it there. Do you agree? Or is this an easy to fix issue? Thanks.
Flags: needinfo?(standard8)
(Assignee)

Comment 7

3 years ago
Created attachment 8653426 [details] [diff] [review]
When obtaining page metadata, always prefer the page title over the opengraph title.

This fixes the issue - ensure's we use the page title, rather than the opengraph title. I've also included some unit tests for this.
Attachment #8653426 - Flags: review?(mixedpuppy)
(Assignee)

Comment 8

3 years ago
(In reply to Ritu Kothari (:ritu) from comment #6)
> Mark, here is another one. I do not consider this release blocking and would
> like to wontfix for FF41 and untrack. We can track it for 42 and consider
> fixing it there. Do you agree? Or is this an easy to fix issue? Thanks.

Patch is now up & simple. However, I'm fine with making this tracking 42, and if it lands in time then uplifting to 41 as well.
Assignee: nobody → standard8
Iteration: --- → 43.2 - Sep 7
Points: --- → 2
Rank: 12
Flags: needinfo?(standard8)
Priority: -- → P1
Attachment #8653426 - Flags: review?(mixedpuppy) → review+
https://hg.mozilla.org/mozilla-central/rev/b3290a39dcba
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Comment 11

3 years ago
Comment on attachment 8653426 [details] [diff] [review]
When obtaining page metadata, always prefer the page title over the opengraph title.

Approval Request Comment
[Feature/regressing bug #]: Context in conversations for Hello.
[User impact if declined]: When attaching context to a conversation, some page titles may be missing information/not have the same title that the user would expect.
[Describe test coverage new/current, TreeHerder]: Has text coverage for the change, landed in m-c.
[Risks and why]: Low for the code change. Context is a recently released feature, and getting the right title is important.
[String/UUID change made/needed]: None

Note: I'm only requesting uplift to aurora as this could have impacts on other areas of the browser using the title. I don't think there will be adverse impacts from what I know of the places this code is used, but just in case, I think it might be better to play safe and give it a full cycle on beta.
Attachment #8653426 - Flags: approval-mozilla-aurora?
status-firefox40: affected → wontfix
Comment on attachment 8653426 [details] [diff] [review]
When obtaining page metadata, always prefer the page title over the opengraph title.

Improve a feature, taking it.
Make sense to only take it in aurora.
Attachment #8653426 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox41: affected → wontfix
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [context] → [context][uxtriage]
See Also: → bug 1252502
Proposal to put back og:title created in Bug 1252502
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.