Closed
Bug 988128
Opened 11 years ago
Closed 11 years ago
[CardView][V1.3T] The icons of card view were broken
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(blocking-b2g:1.3T+, b2g-v1.3 unaffected, b2g-v1.3T verified, b2g-v1.4 unaffected)
VERIFIED
FIXED
blocking-b2g | 1.3T+ |
Tracking | Status | |
---|---|---|
b2g-v1.3 | --- | unaffected |
b2g-v1.3T | --- | verified |
b2g-v1.4 | --- | unaffected |
People
(Reporter: whsu, Assigned: fabrice)
References
Details
(Whiteboard: 1.3tarakorun1 [systemsfe])
Attachments
(3 files)
* Description:
This bug happened on v1.3T build, and cannot reproduce it on other branches.
You will see broken icons after long pressing the home button to show card view.
It happened on following app.
- Dialer app
- Contacts app
-
Attach the screenshot.(The_icons_of_CV.png)
I think it is a side effect of following patch.
- https://github.com/mozilla-b2g/gaia/commit/7ffb9e1ccf0af4d944f8562a0267896cf1c75806
* Reproduction steps:
1. Launch the dialer app or contacts apps
2. Long press the home button
3. Check the card view
* Expected result:
1. Firefox OS displays the card view
2. A icon displays on the center of card view
* Actual result:
The icon was broken
* Reproduction build:(Tarako Build)
- Gaia f6a7afd0d716f7a38dfc1808ee0aab3e06ba2f1d
- Gecko fb7149d9b320d939d716011f865ec34661ebc353
- BuildID 20140326061134
- Version 28.1
Thanks!
Reporter | ||
Updated•11 years ago
|
blocking-b2g: --- → 1.3T?
Whiteboard: 1.3tarakorun1
Reporter | ||
Updated•11 years ago
|
status-b2g-v1.3:
--- → unaffected
status-b2g-v1.3T:
--- → affected
status-b2g-v1.4:
--- → unaffected
Comment 1•11 years ago
|
||
William, can you please confirm whether you have included this Bug 972216?
it should only show icons now.
Minus for now, if you still see problems after including the fix for bug 972216, please renom. thanks
blocking-b2g: 1.3T? → -
Flags: needinfo?(whsu)
Reporter | ||
Comment 2•11 years ago
|
||
Hi, Joe,
As I mentioned on description, this is a side effect of the following commit.
- https://github.com/mozilla-b2g/gaia/commit/7ffb9e1ccf0af4d944f8562a0267896cf1c75806
(Bug 972216)
Thanks!
Flags: needinfo?(whsu)
Comment 3•11 years ago
|
||
(In reply to Joe Cheng [:jcheng] from comment #1)
> William, can you please confirm whether you have included this Bug 972216?
> it should only show icons now.
> Minus for now, if you still see problems after including the fix for bug
> 972216, please renom. thanks
I think that's the entire point of why this bug got filed - the first build that would have it would be the build generated in Taipei's timezone on 3/26 in the morning, so I think he's got the patch. We also didn't anything like what's seen in the screenshot until after bug 972216 landed, so he's definitely got the patch here.
blocking-b2g: - → 1.3T?
Assignee | ||
Comment 5•11 years ago
|
||
I've seen that happening when the app is killed when going to the card view.
Comment 6•11 years ago
|
||
We build the cards view from WindowManager.getRunningApps() and get the icon URL from either app.manifest.icons, or - if there's no manifest - app.icon. And, if neither returns a truthy value we should at least fallback to the default rocket icon which is given in the stylesheet. So the broken icon is odd.
FWIW I'm not able to reproduce this here - but I'm testing on an Open-C.
Flags: needinfo?(sfoster)
Assignee | ||
Comment 7•11 years ago
|
||
Ok, so the bug is at https://github.com/mozilla-b2g/gaia/blob/v1.3t/apps/system/js/cards_view.js#L82
'origin' is not always a real origin because of apps with multiple entry points, so for the contacts app we concatenate app://communications.gaiamobile.org/contacts/index.html + /contacts/style/icons/Contacts_60.png which doesn't lead to a proper uri.
The fix is to do a real url resolution, using 'origin' as a base url.
Assignee | ||
Comment 8•11 years ago
|
||
Sam, that fixes the issue. Do you know on which branch you landed initially and who reviewed?
Flags: needinfo?(sfoster)
Comment 9•11 years ago
|
||
Fabrice, this was bug 972216, it was reviewed by :alive and landed on 1.3t: https://github.com/mozilla-b2g/gaia/commit/f6a7afd0d716f7a38dfc1808ee0aab3e06ba2f1d
Thanks for spotting that, my bad luck that I didn't test with an app with multiple entry points I guess. I'm currently working on the follow-up bug 987237 for master/1.5 (and possible uplift to 1.4) which does this "properly" by toggling the use or screenshot or icon via a setting. (Actually, in that work I had the same origin problem and solution and was going to find out more about it today.)
Flags: needinfo?(sfoster)
Assignee | ||
Updated•11 years ago
|
Attachment #8397315 -
Flags: review?(alive)
Comment 10•11 years ago
|
||
Comment on attachment 8397315 [details] [diff] [review]
card-icon.patch
Review of attachment 8397315 [details] [diff] [review]:
-----------------------------------------------------------------
::: apps/system/js/cards_view.js
@@ +58,4 @@
> */
> function getIconURI(origin) {
> var app = runningApps[origin];
> + if (!app) {
/me slaps head, shoulda caught this before.
@@ +83,5 @@
>
> if (iconPath.indexOf('data:') !== 0) {
> + // We need to resolve iconPath as a relative url to origin, since
> + // origin can be a full url in some apps.
> + var base = document.createElement("a");
There is already a getOriginObject in this file which does exactly this, so we could remove duplication by make this var base = getOriginObject(origin);
@@ +86,5 @@
> + // origin can be a full url in some apps.
> + var base = document.createElement("a");
> + base.href = origin;
> + var port = base.port ? (":" + port) : "";
> + iconPath = base.protocol + "//" + base.host + port + iconPath;
I think base.host should be base.hostname here?
Attachment #8397315 -
Flags: feedback+
Updated•11 years ago
|
blocking-b2g: 1.3T? → 1.3T+
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → fabrice
Updated•11 years ago
|
Attachment #8397315 -
Flags: review?(alive) → review+
Assignee | ||
Comment 11•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 12•11 years ago
|
||
Landed a linter and nit fix as a follow-up: https://github.com/mozilla-b2g/gaia/commit/e96f9b2a481bed8276f8b0a242f39e98c5e3d51a
Reporter | ||
Comment 13•11 years ago
|
||
Thanks for the prompt help!
Verified it.
* Build Information: (V1.3T)
- Gaia 02b97c89dec7a10a955c85b921b2a66181eebb0e
- Gecko f8bca24057937f5b09d73512ba1e771011ab4203
- BuildID 20140331124355
- Version 28.1
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 14•11 years ago
|
||
Reporter | ||
Comment 15•11 years ago
|
||
attach the screenshot (CardView.png)
You need to log in
before you can comment on or make changes to this bug.
Description
•