Closed
Bug 826400
Opened 12 years ago
Closed 9 years ago
Use apple-touch-icon for homescreen shortcuts
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox46 fixed, relnote-firefox 46+)
RESOLVED
FIXED
Firefox 46
People
(Reporter: wesj, Assigned: ahunt)
References
Details
(Keywords: dev-doc-needed)
Attachments
(4 files)
I think we need to do a bit better at looking for high res art, both for thumbnails in our product and homescreen shortcuts. Junior uses some nice icons that are hardcoded, but pages provide some. Some examples:
1.) many sites have high res art in random meta tags (qz.com lists http://app.qz.com/img/qz_og_img.png in a twitter card and opengraph data). These might be useful for about:home thumbnails, tab thumbnails (using images for twitter cards?), and as shortcut icons. How do we feel about creating a white list of tags that we're comfortable using and prioritizing them?
2.) RSS feeds (where NYT links to http://graphics8.nytimes.com/images/misc/NYT_logo_rss_250x40.png). These tend to be less square, but are often much more detailed.
Reporter | ||
Comment 1•12 years ago
|
||
Another example: CNN provides a
<meta name="msapplication-TileImage" content="http://i.cdn.turner.com/cnn/2012/images/10/15/cnn_logo_144_144.png"/>
along with a TileColor for Windows 8. Wonder if we could share a "parser" with the Metro team.
Comment 3•11 years ago
|
||
<link rel="apple-touch-icon"> are probably the most widely used format for homescreen shortcut icons. Maybe search for any images in link/meta tags inside <head> and take the biggest of those where x=y?
Comment 4•11 years ago
|
||
Fixed by Bug 914296 and 748100. Pretty picture:
https://bugzilla.mozilla.org/attachment.cgi?id=800002
(Top row with, bottom row without)
(Assuming the solution you're after is "Use higher resolution Favicons).
Assignee: nobody → ckitching
Updated•11 years ago
|
Assignee: chriskitching → nobody
Blocks: fennec-polish
Comment 8•9 years ago
|
||
Google has started pushing a WebApp Manifest, which supports a set of high resolution assets. We should consider looking for the manifest. Gecko has code to read a manifest:
http://mxr.mozilla.org/mozilla-central/source/dom/manifest/ManifestObtainer.jsm
Updated•9 years ago
|
Summary: Try harder to find useful art for thumbnails and homescreen shortcuts → Try harder to find useful art for homescreen shortcuts
Updated•9 years ago
|
Comment 9•9 years ago
|
||
Does this also include having a sensible fallback as well?
For example, if the icon isn't "useful enough"/of a certain size, could we wrap it around a nicer button UI?
Comment 10•9 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #9)
> Does this also include having a sensible fallback as well?
>
> For example, if the icon isn't "useful enough"/of a certain size, could we
> wrap it around a nicer button UI?
We already have logic to do that with the dominant color, but that could potentially stand to be improved. If you have ideas about what we could do to make that fallback better, I would file a new bug about it (and link it back to bug 1212648).
Comment 11•9 years ago
|
||
What I'm hearing is that there a number of places websites stick their art that we could use, so maybe let's pick one and see how much of a win that gets us. https://bugzilla.mozilla.org/show_bug.cgi?id=826400#c3 names apple-touch-icons so why don't we start there.
Updated•9 years ago
|
Assignee: nobody → ally
Updated•9 years ago
|
Assignee: a.m.naaktgeboren → nobody
Updated•9 years ago
|
Assignee: nobody → ahunt
Comment 12•9 years ago
|
||
Would it be possible to reuse the design in bug 1228680?
This would keep it consistent and help with recall but also be more useful as a fallback when we don't have perfectly nice icons.
Comment 13•9 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #12)
> Would it be possible to reuse the design in bug 1228680?
>
> This would keep it consistent and help with recall but also be more useful
> as a fallback when we don't have perfectly nice icons.
I think that would be a different bug. This bug is not about creating a new fallback. This one is about finding better images so we can avoid the fallback.
The fallback code is a separate, more narrow scope.
Assignee | ||
Comment 14•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28097/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28097/
Assignee | ||
Comment 15•9 years ago
|
||
I've got this working for the apple-touch-icon now - see preliminary patch above.
There are still some issues I need to address:
1. Using the "sizes" attribute - makeFavIconMessage parses the sizes for us, however we discard that and overwrite touchIcon with whichever apple-touch-icon was listed last in the page source. (For favicons, each size is sent separately to Tab, and later handled in the Favicon code which lets us retrieve different sizes as needed.)
It would probably be simplest to filter the sizes during pageload, and store just one touchIcon url, however I don't think we have access to the preferred home screen icon size at that point - this might require more work on the database side to store multiple sizes, and then filter them when creating the homescreen shortcut? (That would probably needed for app manifests that provide multiple icon sizes too.)
2. Fix the URLMetadataTable caching. Currently we push data into the cache on any request to LocalURLMetaData - regardless of what columns were actually requested (and thus cached). E.g. on startup we retrieve tileImage/tileColor for all top-sites (for the top-sites panel), if we then try to add a top-site to the home screen, we have a cache hit, but the result doesn't contain the touchIcon. I've added an ugly check to re-retrieve rows with missing data to the current patch.
I think it makes most sense to enable the cache only on requests for tileImage+tileColor, and not bother caching touchIcon requests (the former are retrieved every time we show the top-sites panel, the latter is used rarely), but that would make the code quite a bit more complex. Alternatively we could just cache all columns (but that means we'd need to retrieve all columns on every request).
Updated•9 years ago
|
Attachment #8698716 -
Flags: feedback?(nalexander)
Comment 16•9 years ago
|
||
https://reviewboard.mozilla.org/r/28097/#review25183
::: mobile/android/chrome/content/browser.js:4195
(Diff revision 1)
> + // TODO: we can have multiple sizes of touch icons (similary to favicons) - handle that somehow?
I think we hold the metadata until the page is loaded. Once loaded, we send the "best match" to Java.
We could use: this.METADATA_GOOD_MATCH + message.size
That would make bigger sizes preferred.
Comment 17•9 years ago
|
||
https://reviewboard.mozilla.org/r/28097/#review25239
::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1230
(Diff revision 1)
> - Integer.MAX_VALUE,
> + GeckoAppShell.createShortcut(title, url);
nit: indentation is off.
::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:755
(Diff revision 1)
> - static void createShortcut(final String aTitle, final String aURI, final String aIconData) {
> + public static void createShortcut(final String aTitle, final String aURI) {
My expectation is that this will fail when you build it in automation, since this is an entrypoint accessed from the compiled C++ code.
This is a limitation of artifact builds. For testing, keep the signature the same and ignore the parameter. For landing, you'll need to address consumers, like https://dxr.mozilla.org/mozilla-central/source/mobile/android/components/build/nsShellService.cpp#28.
::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:759
(Diff revision 1)
> - Favicons.getSizedFavicon(getApplicationContext(), aURI, aIconData, Integer.MAX_VALUE, 0,
> + // The columns list will be mutated by metadata, hence we can't use Collections.singletonList
Consider making the implementation not require this, since it's almost never what you want. That is, make the method copy the list if needed and add the URL column.
::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:764
(Diff revision 1)
> + final Map<String, Map<String, Object>> metadata = db.getURLMetadata().getForURLs(cr,
Wow, this API is funky.
::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:771
(Diff revision 1)
> + String iconURL = null;
nit: touchIconURL. Let's be explicit.
::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:788
(Diff revision 1)
> + Favicons.getSizedFavicon(getApplicationContext(), aURI, iconURL, Integer.MAX_VALUE, 0, listener
nit: The indentation and line breaks here some off.
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:117
(Diff revision 1)
> - // Cache hit!
> + // Cache hit - however the cache might not contain all the needed columsn. E.g. the top sites panel
nit: columsn.
This comment needs some editorial love. Full sentences, please.
Explain why this is ugly and explain how these things would improve matters rather than stating that it is ugly.
::: mobile/android/base/java/org/mozilla/gecko/home/HomeFragment.java:225
(Diff revision 1)
> + GeckoAppShell.createShortcut(info.getDisplayTitle(), info.url);
There's a slight change in semantics here, since before we captured the title earlier. Perhaps do that?
::: mobile/android/chrome/content/browser.js:4195
(Diff revision 1)
> + // TODO: we can have multiple sizes of touch icons (similary to favicons) - handle that somehow?
nit: similary.
I think we should try to surface all the relevant metadata to the Java table, and do the filtering on that side, where we have the most complete information.
Nice patch! I have some nits, a structural request, and a substantive request.
Structural request: the smaller your patches, the easier they are to review. For this patch, split it into two parts: one, database changes (preserving existing functionality); two, the meat. Here, they're intertwingled.
Substantive request: the API at https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java#97 is pretty rough. As a part 0, make that not mutate its inputs (and update callers).
Larger question: Are we really gaining anything from this general system of `Map<String, *>`, considering this is all getting backed by actual SQL columns?
Comment 18•9 years ago
|
||
Comment on attachment 8698716 [details]
MozReview Request: Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
I left comments via RB.
Attachment #8698716 -
Flags: feedback?(nalexander) → feedback+
Assignee | ||
Comment 19•9 years ago
|
||
Comment on attachment 8698716 [details]
MozReview Request: Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28097/diff/1-2/
Attachment #8698716 -
Attachment description: MozReview Request: Bug 826400 - use apple-touch-icon for homescreen shortcuts → MozReview Request: Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
Attachment #8698716 -
Flags: feedback+ → review?(nalexander)
Assignee | ||
Comment 20•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28821/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28821/
Attachment #8700871 -
Flags: review?(nalexander)
Assignee | ||
Comment 21•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28823/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28823/
Attachment #8700872 -
Flags: review?(nalexander)
Assignee | ||
Comment 22•9 years ago
|
||
I've just pushed updated/split patches (see above)! Thank you for the review.
Some notes:
In the current implementation we grab all touchIcon sizes on the JS side, send the list to the Java side, filter these sizes (based on Android's preferred homescreen icon size), and finally write one optimal icon size to the DB - this icon is then used for homescreen shortcut creation.
We've discussed storing all the sizes in the DB, however currently the only use for these icons is homescreen/launcher icons - these have a fixed size for any given device, hence storing one size should be sufficient here.
We could potentially use the multiple touchIcon sizes to improve other parts of the browser ("top sites, rich history visits, better sharing UX, notifications") - however I'm not sure this is sensible given that some sites (e.g. twitter) use completely different designs for their favicons vs touchIcons (the favicon has a clear background / is optimised for showing on a white surface, whereas the touchIcon has a solid black background and seems to be optimised for being part of a launcher). It's already possible for sites to supply larger favicons (and if I understand the code correctly, we will use those as appropriate) - it seems like we're more likely to make the experience worse if we try to use the touchIcon outside of the homescreen?
Assignee | ||
Comment 23•9 years ago
|
||
Comment on attachment 8698716 [details]
MozReview Request: Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28097/diff/2-3/
Assignee | ||
Comment 24•9 years ago
|
||
Comment on attachment 8700871 [details]
MozReview Request: Bug 826400 - Part 2: Extract and store optimal apple-touch-icon during page load r=nalexander
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28821/diff/1-2/
Assignee | ||
Comment 25•9 years ago
|
||
Comment on attachment 8700872 [details]
MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28823/diff/1-2/
Updated•9 years ago
|
Summary: Try harder to find useful art for homescreen shortcuts → Use apple-touch-icon for homescreen shortcuts
Updated•9 years ago
|
Attachment #8698716 -
Flags: review?(nalexander) → review+
Comment 27•9 years ago
|
||
Comment on attachment 8698716 [details]
MozReview Request: Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
https://reviewboard.mozilla.org/r/28097/#review25763
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:146
(Diff revision 3)
> + List<String> columns = requestedColumns;
OK.
wfm. Do we have JUnit 3 unit tests of this DB? If not, please file a ticket to exercise the functionality.
Updated•9 years ago
|
Attachment #8700871 -
Flags: review?(nalexander)
Comment 28•9 years ago
|
||
Comment on attachment 8700871 [details]
MozReview Request: Bug 826400 - Part 2: Extract and store optimal apple-touch-icon during page load r=nalexander
https://reviewboard.mozilla.org/r/28821/#review25767
nits, including one substantive request. Be careful with the JSON types -- you haven't really guaranteed string values and integer (or long) keys. (In this situation, I don't think you need to guarantee it, but you do need to handle bad inputs.)
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:83
(Diff revision 2)
> + // Try to select the closest icon that is larger than (or equal to) the target size - this
Move this comment up and include a sketch of what the data looks like. Something like:
// Incoming data should look like: {touchIconList:{100:"http://x.com/100.png", 200:"http://x.com/200.png"}}.
And then explain your strategy.
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:85
(Diff revision 2)
> + if ((iconSize < preferredSize && iconSize > bestSizeFound) ||
This is hard to read. Can you express it more simply? I think I'd sort the list in ascending order, and then look for the cases sequentially: i.e., find the first icon >= than the preferred size; if there isn't one, just return the last size icon (it's the largest, and it's smallest overall.)
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:91
(Diff revision 2)
> + } catch (JSONException e) {
This isn't particularly robust, since some icon sizes may be malformed and other sizes may be well formed. Since we produce the incoming data, that's probably fine.
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:92
(Diff revision 2)
> + e.printStackTrace();
Use `Log.w(LOGTAG, message, e)`. This shouldn't be fatal -- you pretty much have to ignore it and move on.
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:95
(Diff revision 2)
> + Assert.isNotNull(iconURL);
What do you want to do on failure? Assert's don't crash in beta or release, so prefer to log and adapt rather than possibly crash.
::: mobile/android/chrome/content/browser.js:3942
(Diff revision 2)
> - if (!this.metatags[type] || this.metatags[type + "_quality"] < quality) {
> + if (type === "touchIconList") {
I don't think we use `===` often. Just `==`.
::: mobile/android/chrome/content/browser.js:3944
(Diff revision 2)
> + this.metatags['touchIconList'] = {};
Consider setting metatags['touchIconList_quality'] as well. (Since we're changing the API.)
Updated•9 years ago
|
Attachment #8700872 -
Flags: review?(nalexander) → review+
Comment 29•9 years ago
|
||
Comment on attachment 8700872 [details]
MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
https://reviewboard.mozilla.org/r/28823/#review25773
Fine by me, but let's get snorp's stamp.
::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1107
(Diff revision 2)
> - Bitmap favicon = tab.getFavicon();
> + final String favicon = tab.getFaviconURL();
It looks like `favicon` is unused: remove it.
::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:756
(Diff revision 2)
> - static void createShortcut(final String aTitle, final String aURI, final String aIconData) {
> + public static void createShortcut(final String aTitle, final String aURI) {
Ah, I see this. I'm going to redirect to snorp for two questions:
1) is this OK, since I don't know the JNI interfaces well;
2) are we confident that we can bump nsIShellService like this? (It may be used by other parts of the code base and also add-ons.)
::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:762
(Diff revision 2)
> + Collections.singletonList(URLMetadataTable.TOUCH_ICON_COLUMN)
Oh, neat -- I wasn't aware of this helper.
::: mobile/android/components/build/nsIShellService.idl:22
(Diff revision 2)
> * @param aIconData a base64-encoded data: URI representation of the shortcut's icon, as accepted by the favicon decoder.
Update the comment, please.
Comment 30•9 years ago
|
||
Comment on attachment 8700872 [details]
MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
snorp, jchen: can one of you vet the JNI changes here? It's not clear to me that we can bump this IDL without repurcussions.
Attachment #8700872 -
Flags: review?(snorp)
Attachment #8700872 -
Flags: review?(nchen)
Comment 31•9 years ago
|
||
Comment on attachment 8700872 [details]
MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
https://reviewboard.mozilla.org/r/28823/#review25949
::: mobile/android/components/build/nsIShellService.idl:25
(Diff revision 2)
> - void createShortcut(in AString aTitle, in AString aURI, in AString aIconData, in AString aIntent);
> + void createShortcut(in AString aTitle, in AString aURI, in AString aIntent);
You should change the UUID as well if you make a non-trivial change to the IDL interface.
I couldn't find our code actually using nsIShellService, but this change could affect addons. The safer way is probably to deprecate the parameter but keep it there, like what we did for the aIntent parameter.
Attachment #8700872 -
Flags: review?(nchen)
Assignee | ||
Comment 32•9 years ago
|
||
https://reviewboard.mozilla.org/r/28821/#review25767
> This is hard to read. Can you express it more simply? I think I'd sort the list in ascending order, and then look for the cases sequentially: i.e., find the first icon >= than the preferred size; if there isn't one, just return the last size icon (it's the largest, and it's smallest overall.)
I've implemented this, I'm just not sure I'm doing this in a sane way:
My code iterates over the list keys (JSONObject.keys(), which gives us an Iterator) to copy them into a sortable ArrayList - that feels a bit clunky to me, but there doesn't seem to be any better way of getting a sortable list from the JSONObject - does that seem right?
> Consider setting metatags['touchIconList_quality'] as well. (Since we're changing the API.)
Would you want this to be an additional list of qualities/sizes (touchIconList_quality:{"200","300"}), or to replace the current storage of size within touchIconList (so touchIconList:{"x.com/200.png", "x.com/300.png"}, touchIconList_quality:{"200","300"}), or something else (e.g. storing just the largest quality obtained so far)?
(I'd be worried about splitting the list into separate icon and size lists, especially since we're going to need a map from size->icon later - I'm not sure if that's what you want?)
Assignee | ||
Comment 33•9 years ago
|
||
Comment on attachment 8698716 [details]
MozReview Request: Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28097/diff/3-4/
Assignee | ||
Comment 34•9 years ago
|
||
Comment on attachment 8700871 [details]
MozReview Request: Bug 826400 - Part 2: Extract and store optimal apple-touch-icon during page load r=nalexander
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28821/diff/2-3/
Attachment #8700871 -
Flags: review?(nalexander)
Assignee | ||
Comment 35•9 years ago
|
||
Comment on attachment 8700872 [details]
MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28823/diff/2-3/
Attachment #8700872 -
Attachment description: MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander → MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
Attachment #8700872 -
Flags: review?(snorp) → review?(nchen)
Comment 36•9 years ago
|
||
Comment on attachment 8700872 [details]
MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
https://reviewboard.mozilla.org/r/28823/#review26049
Attachment #8700872 -
Flags: review?(nchen) → review+
Updated•9 years ago
|
Attachment #8700871 -
Flags: review?(nalexander) → review+
Comment 37•9 years ago
|
||
Comment on attachment 8700871 [details]
MozReview Request: Bug 826400 - Part 2: Extract and store optimal apple-touch-icon during page load r=nalexander
https://reviewboard.mozilla.org/r/28821/#review26421
lgtm. Sorry for the delay!
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:92
(Diff revisions 2 - 3)
> - // should hopefully give us the highest quality favicon possible
> + for (int size: sizes) {
nit: house style is to space the colon -- like:
```
for (int size : sizes) {
}
```
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:102
(Diff revisions 2 - 3)
> + bestSizeFound = sizes.get(sizes.size() - 1);
Guard against empty collections here.
::: mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java:107
(Diff revisions 2 - 3)
> - e.printStackTrace();
> + Log.w(LOGTAG, "Exception processing touchIconList for LocalURLMetadata", e);
The important thing is to log what you're doing, to save passers-by reading the code:
"for ...; ignoring."
Comment 38•9 years ago
|
||
https://reviewboard.mozilla.org/r/28821/#review25767
> I've implemented this, I'm just not sure I'm doing this in a sane way:
>
> My code iterates over the list keys (JSONObject.keys(), which gives us an Iterator) to copy them into a sortable ArrayList - that feels a bit clunky to me, but there doesn't seem to be any better way of getting a sortable list from the JSONObject - does that seem right?
Sadly, I don't see `.keySet`, which would be better :(
Assignee | ||
Comment 39•9 years ago
|
||
Comment on attachment 8698716 [details]
MozReview Request: Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28097/diff/4-5/
Assignee | ||
Comment 40•9 years ago
|
||
Comment on attachment 8700871 [details]
MozReview Request: Bug 826400 - Part 2: Extract and store optimal apple-touch-icon during page load r=nalexander
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28821/diff/3-4/
Assignee | ||
Comment 41•9 years ago
|
||
Comment on attachment 8700872 [details]
MozReview Request: Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28823/diff/3-4/
Assignee | ||
Comment 42•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0bbc5f3f9c671de8b9d6a2a054e226ecc4b3398e
Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
https://hg.mozilla.org/integration/fx-team/rev/f21ce36d052486948d2712a2632612e7fa6a506f
Bug 826400 - Part 2: Extract and store optimal apple-touch-icon during page load r=nalexander
https://hg.mozilla.org/integration/fx-team/rev/cc103b8b1009effb6e4cc576174b70c0153c0b23
Bug 826400 - Part 3: Simplify homescreen shortcut creation, and use apple-touch-icon if available r=nalexander,jchen
Comment 43•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0bbc5f3f9c67
https://hg.mozilla.org/mozilla-central/rev/f21ce36d0524
https://hg.mozilla.org/mozilla-central/rev/cc103b8b1009
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Comment 44•9 years ago
|
||
Hi,
I tested it with the Nightly from 2015-01-10 on my HTC One A9 with Android 6.0 and my blog www.soeren-hentzschel.at - it does not work, the apple-touch-icon is not used as home screen icon.
Comment 45•9 years ago
|
||
Strange. It seems to be working for me when adding Twitter to my home screen. Might be worth opening a new bug.
Assignee | ||
Comment 46•9 years ago
|
||
(In reply to Sören Hentzschel from comment #44)
> I tested it with the Nightly from 2015-01-10 on my HTC One A9 with Android
> 6.0 and my blog www.soeren-hentzschel.at - it does not work, the
> apple-touch-icon is not used as home screen icon.
The issue here is that we can't load icons from sites needing SNI for their SSL cert. See Bug 507641 for the main bug, there's also some discussion about http libraries in Bug 765064.
As a result we can't load both favicons and touchIcons for your site (or any SNI using site), meaning we just show the default star on the homescreen, and also don't show a favicon during normal browsing.
Updated•9 years ago
|
OS: Linux → Android
Hardware: x86 → All
Assignee | ||
Comment 48•9 years ago
|
||
I've just learned that we should be updating the Bug number beside DATABASE_VERSION, here's a patch that does that, and also adds an explanation to hopefully avoid this not happening in future.
Attachment #8717043 -
Flags: review?(liuche)
Updated•9 years ago
|
Attachment #8717043 -
Flags: review?(liuche) → review+
Assignee | ||
Comment 49•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/de8bfa116f66406ac3ca874a1eb4f560057cc7d4
Bug 826400 - Post: update DATABASE_VERSION comment, and clarify why we do this r=liuche
Comment 50•9 years ago
|
||
bugherder |
Updated•8 years ago
|
Keywords: dev-doc-needed
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•