Closed
Bug 1388396
Opened 7 years ago
Closed 7 years ago
Activity Stream: when have no good favicon, display letter from subdomain over stable color
Categories
(Firefox for Android Graveyard :: General, enhancement, P1)
Tracking
(firefox57 verified)
VERIFIED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | verified |
People
(Reporter: sebastian, Assigned: mcomella)
References
Details
(Whiteboard: [MobileAS])
Attachments
(9 files, 1 obsolete file)
488.55 KB,
image/png
|
Details | |
213.11 KB,
image/png
|
Details | |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
205.00 KB,
image/png
|
Details | |
59 bytes,
text/x-review-board-request
|
mcomella
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
mcomella
:
review+
|
Details |
Some sites only give us tiny favicons. Currently we just center them in the top sites tile and use an extracted background color (see attached screenshot). This doesn't look nice. What else could we do here? We already generate "nicer" icons if the website doesn't provide one - should we generate one too when the icon is too small?
Assignee | ||
Comment 1•7 years ago
|
||
bbell mentioned that if we can't get a high-res enough icon, we can display screenshots (see also bug 1389259, which is screenshots for a different purpose).
Reporter | ||
Comment 2•7 years ago
|
||
I wonder if we could still run into situations where we do not have screenshots and just a tiny favicon?
Comment 3•7 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #2) > I wonder if we could still run into situations where we do not have > screenshots and just a tiny favicon? Almost certainly. We drop screenshots when we try to reduce storage and memory pressure, they can fail to be captured, and so on.
Assignee | ||
Comment 4•7 years ago
|
||
Spoke with bbell: we should: 1) Show a screenshot if possible (related: bug 1389259) 2) Show a letter from the subdomain, if available; other domain & surround it with a stable color so that if the domain/subdomain appears in the list multiple times, they'll have the same color (it doesn't have to extract a color from the tiny favicon, especially because it can be ugly; we probably have the code to do all this already) As Sebastian mentions in comment 5, it's possible we'll have no screenshot so we'll always need #2 so I think we should start there and maybe add screenshots later as an optimization: repurposing this bug as such.
Summary: Activity Stream: How to handle tiny favicons? → Activity Stream: when have no good favicon, display letter from subdomain over stable color
Assignee | ||
Comment 5•7 years ago
|
||
We'll handle screenshots in bug 1389259.
Assignee | ||
Comment 6•7 years ago
|
||
We load icons in IconTask.loadIcons. When all of the loaders fail, we call IconGenerator.generate [1]. To fix this bug, we'll need to add a branch such that if the loaded icon is too small, we fall back onto IconGenerator.generate). I assume we want to fall back to generating (rather than running the other loaders) to avoid the case where we hit the cache, find the icon is too small, download a new image, it's the same icon (because there are very low chances it'll be larger), and we end up hitting the network each time we want to see this image even though, again, the icon is almost certainly the same. We may want to add a configuration option for bailing out for small icons, in IconRequestBuilder. [1]: https://dxr.mozilla.org/mozilla-central/rev/a9d372645a32b8d23d44244f351639af9d73b96a/mobile/android/base/java/org/mozilla/gecko/icons/IconTask.java#140
Assignee | ||
Comment 7•7 years ago
|
||
Please feel free to take this bug - I'm going to work on bug 1391422 because it's a regression from my previous changes.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → michael.l.comella
Assignee | ||
Comment 8•7 years ago
|
||
Some notes: - In IconTask, we process all favicons closer to the desired size (favicon_bg, currently 112dp). This occurs *after* we load the icon (which means the second time we load the icon, the icon we load will be larger - tricky!). - If increasing in size, we'll only resize a max of 2x - In bug 1388379 comment 9, I determined the max size of favicon we show is 90dp, based on the top sites tile size Sebastian, do you know why there's a discrepancy between the top sites tile size (90dp) and the size we generate our favicons to (112dp)?
Flags: needinfo?(s.kaspari)
Assignee | ||
Comment 9•7 years ago
|
||
More notes: - The width and height values of the Bitmap in the IconResponse is in pixel values - We'll need an algorithm to determine when an icon is too small. Since our top sites items are dynamically sized and icon's size will change based on the device size, we may wish to set our cutoff value as a percentage of the actual container size rather than an absolute value. - We annoying thing: the highlights and top sites containers are different sizes but we'll want to make sure the icons are consistently used between them --- Aaron, Bryan: do you have an opinion on how we might determine when top sites/highlight icon is too small such that we want to replace it with either a screenshot (bug 1389259) or a generated favicon? I've attached a screenshot of some small favicons for a reference point. In my opinion, in the screenshot, I think we should replace waitbutwhy (highlights) and keep all the others but I could go either way on Yahoo. For my opinion, that means we would replace icons that are roughly less than 1/3 the width of the top sites/highlight container.
Flags: needinfo?(bbell)
Flags: needinfo?(abenson)
Assignee | ||
Updated•7 years ago
|
Attachment #8900537 -
Attachment is obsolete: true
Assignee | ||
Comment 10•7 years ago
|
||
I previously attached the wrong screenshot.
Reporter | ||
Comment 11•7 years ago
|
||
A naive approach might be to ignore an IconResponse object in IconTask.loadIcon() if it's considerably smaller than IconRequest.targetSize (as you mentioned upscaling happens later - so we should take this into account). Whether we want to do this or not could be a configuration detailed set in IconRequest. I guess we do not want to do this everywhere in our app.
Updated•7 years ago
|
Rank: 1
Assignee | ||
Comment 12•7 years ago
|
||
Spoke with bbell on vidyo: we want to have the favicons always fill the top sites tiles and thus no longer want to center favicons with a colored margin. We'll need to experiment with the parameters but note that bbell prefers a slightly fuzzy icon over a letter. For the first experiment: - if the icon is 90px square or larger, increase the size so it fills the tile. If there isn't too much artifacting, let's keep the implementation, otherwise follow up with bbell! - If we don't have a 90px icon, fall back on screenshots (bug 1389259) then the favicon with the letter. Follow-up note: bbell mentioned there are probably enough 128px square icons for this to work so we should try that value next (rather than using the more round, relative to our image size, 180px square).
Flags: needinfo?(bbell)
Flags: needinfo?(abenson)
Assignee | ||
Comment 13•7 years ago
|
||
Side note: if we're removing the centered-favicon with colored margin from top sites, I wonder if we should remove this from all of our UI (since top sites currently has the largest icons of them all).
Assignee | ||
Comment 14•7 years ago
|
||
Not prioritized for the current sprint and I have other things to focus on: unassigning.
Assignee: michael.l.comella → nobody
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → michael.l.comella
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(s.kaspari)
Assignee | ||
Comment 15•7 years ago
|
||
Note: after investigating distributions (bug 1394641), to support our old users, we'll always need to be able to read the bgcolor property and draw a background color behind an icon, even if we don't do that for our code.
Assignee | ||
Comment 16•7 years ago
|
||
The problem is a different bitmap can be cached to disk based on who calls first: - It won't have a resize cap if AS calls first and will have artifacting, even when downscaling for history items - It'll be capped at 2x scale otherwise, which AS won't show because it's too small (when we have a minimum size) This resizing bitmap is cached and returned for every other call load in icons so we get different results in *all* views based on who called first. Quick thoughts on solutions: - Have separate disk caches based on resizes - Cache the original asset, not the resized asset - Use the same icon algorithm everywhere wrt resizing & minimum sizes. Ideally, I think this should be the case but I'm concerned how far reaching this might be and that e.g. history icons appear more slow and can take lower resolution images Review commit: https://reviewboard.mozilla.org/r/177904/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/177904/
Assignee | ||
Comment 17•7 years ago
|
||
The problem here is that if AS first calls the icon, it'll get the original icon size which it may reject (e.g. 64px). However, somewhere else will request the icon, resize it 2x (128px), and cache that result. The next time AS calls, it'll get the resized icon and display it. Thoughts on solutions: - It may be comprehensive with the previous issue - We figure out how to get the original image size from the cache rather than the resized size Review commit: https://reviewboard.mozilla.org/r/177906/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/177906/
Assignee | ||
Comment 18•7 years ago
|
||
I'm having difficulty with this - for details see comment 16 and comment 17 above. For a high level summary, the icon cache is storing resized images but we're setting different resizing criteria for different views [1], so different bitmaps are cached depending on who calls it first and since all subsequent loads pull from the cache, it affects all icons in the app. [1]: I didn't want to change how our icons work comprehensively because I'm afraid of the larger effects (e.g. when Gecko requests an icon, do we really want to return one with the same criteria we use for Top Sites?). Basically, as per comment 12, AS has no resize cap and will discard images below a certain size while other views will only resize 2x and do not discard icons.
Assignee | ||
Comment 19•7 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #16) > Quick thoughts on solutions: > ... > - Use the same icon algorithm everywhere wrt resizing & minimum sizes. > Ideally, > I think this should be the case but I'm concerned how far reaching this might > be and that e.g. history icons appear more slow and can take lower resolution > images I just wrote a patch and the few pages I have in my history don't have favicons at 64px+ and appear with generated icons, for example: - bugzilla.mozilla.org - addons.mozilla.org (a default bookmark) - waitbutwhy - gmx, a distribution - yahoo redirect links - blog.bugzilla.com (a vw blog) This seems really unfortunate. Another (hacky!) solution thought: we can add an activityStreamProcessor after the ResizingProcessor which will fall back to generated icons if the resized icons are too small. This is hacky because it takes advantage of the specific implementation. Another thing I noticed: the disk cache doesn't have the resized image but the memory cache does. --- Sebastian, how would you approach this (what I'm trying to accomplish is in comment 12)? Are you seeing something I don't?
Flags: needinfo?(s.kaspari)
Updated•7 years ago
|
Iteration: --- → 1.30
Updated•7 years ago
|
Rank: 1
Priority: P2 → P1
Assignee | ||
Comment 20•7 years ago
|
||
Just a thought: I'm coming at this looking for the perfect solution but perhaps we can get a good enough solution. e.g. I'm trying to both remove colored backgrounds and fix tiny favicons but we could just remove tiny favicons in top sites and do a separate bug for colored backgrounds.
Assignee | ||
Comment 21•7 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #20) > e.g. I'm trying to both remove colored backgrounds bug 1398863.
Assignee | ||
Comment 22•7 years ago
|
||
I'm concerned the plan in comment 12 will not work because we show too many placeholder icons (bug 19). However, it's hard to know for sure because the random sites I visit are not real profiles. After speaking with Chenxia, I came up with a "good enough" approach: we discard the smallest images that are not worth scaling up and scale up the medium sized images just so that they're large enough to fill the top site space well and not display too much artifacting. I found some decent parameters by testing a few different favicons on a few devices. I confirmed with Maria that she'd prefer the slightly artifacted, centered icons over the placeholder icons the original approach would encourage. Due to time constraints, I'm going with the good-enough approach and I filed bug 1398970 to follow-up with a better implementation.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 27•7 years ago
|
||
Here's what top sites would look like: the top row are fairly high res icons while the bottom row are scaled up 3x & generated. Take any artifacting in the screenshot with a grain of salt: this change is really about how it looks on device, not how the screenshot looks on a computer. The screenshot should be useful for comparing the overall positioning of icons. github.com/accounts.google.com are the smallest icons we'll show and they're originally 32px scaled up to 96px.
Assignee | ||
Comment 28•7 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #8) > Sebastian, do you know why there's a discrepancy between the top sites tile > size (90dp) and the size we generate our favicons to (112dp)? Looks like this is because highlights/pocket have a width of 112dp and we use smaller size (based on a ratio) for the height.
Flags: needinfo?(s.kaspari)
Assignee | ||
Comment 29•7 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #22) > I confirmed with Maria that she'd prefer the slightly artifacted, centered > icons over the placeholder icons the original approach would encourage. That is because the placeholder icons are more usable and recognizable by users than the placeholders.
Assignee | ||
Comment 30•7 years ago
|
||
One thought: one way of testing this is to land it and see if anyone files a, "This looks awful" bug in response. :)
Assignee | ||
Comment 31•7 years ago
|
||
To be explicit, I tested on a Nexus 7 (xhdpi), a Nexus 5 (xxhdpi), and a Nexus S (? but I'm guessing hdpi). However, the result is a relationship between the screen size, the DPI, and honestly the display quality so my testing is by no means comprehensive.
Reporter | ||
Comment 32•7 years ago
|
||
mozreview-review |
Comment on attachment 8906169 [details] Bug 1388396: Increase max favicon scale factor to 3. https://reviewboard.mozilla.org/r/177904/#review183802 ::: mobile/android/base/java/org/mozilla/gecko/icons/processing/ResizingProcessor.java:23 (Diff revision 2) > + // This is the largest factor we'll scale up an image by: the goal is an image > + // that both fills the top site space well but does not have extreme resizing > + // artifacts. This number was chosen anecdotally by comparing variously-sized > + // favicons across devices to see which factor(s) looked the best. bug 1398970 > + // is filed to take a more comprehensive approach to favicons. > + public static final int MAX_SCALE_FACTOR = 3; I wonder whether this value should depend on the device density. In the UI we display icons based on a physical size (dp) - but here we are scaling pixels. But that's something for bug 1398970 to think about..
Attachment #8906169 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Comment 33•7 years ago
|
||
mozreview-review |
Comment on attachment 8906170 [details] Bug 1388396: Add IconRequestBuilder.forActivityStream. https://reviewboard.mozilla.org/r/177906/#review183804 ::: mobile/android/base/java/org/mozilla/gecko/icons/IconRequestBuilder.java:16 (Diff revision 2) > import org.mozilla.gecko.GeckoAppShell; > > import java.util.TreeSet; > > import ch.boye.httpclientandroidlib.util.TextUtils; > +import org.mozilla.gecko.icons.processing.ResizingProcessor; nit: This is class does not exist yet. :) ::: mobile/android/base/java/org/mozilla/gecko/icons/IconRequestBuilder.java:112 (Diff revision 2) > + public IconRequestBuilder forActivityStream() { > + // This value was set anecdotally: 16px icons scaled up both look blurry and > + // don't fill the space well. 32px icons look good enough. > + internal.minimumSizePxAfterScaling = 32 * ResizingProcessor.MAX_SCALE_FACTOR; > + return this; > + } I like wrapping the configuration in a for*() method. I was a bit concerned about adding this directly to the icon code at first. I would like this code to be mostly independent - like a library for loading icons. But then I saw that I added already forLauncherIcon() and some of the loaders are very specific to Fennec already too. So I guess that's not something worth separating right now. :)
Attachment #8906170 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Comment 34•7 years ago
|
||
mozreview-review |
Comment on attachment 8906867 [details] Bug 1388396: Use IconRequestBuilder.forActivityStream in AS UI. https://reviewboard.mozilla.org/r/178600/#review183806
Attachment #8906867 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Comment 35•7 years ago
|
||
mozreview-review |
Comment on attachment 8906868 [details] Bug 1388396: Add MinimumSizeProcessor; .forActivityStream takes effect. https://reviewboard.mozilla.org/r/178602/#review183808 ::: mobile/android/base/java/org/mozilla/gecko/icons/processing/MinimumSizeProcessor.java:23 (Diff revision 1) > + * {@link ResizingProcessor} the second time, when it's loaded from the cache. It turned out to be much simpler > + * to enforce the requirement that... > + * > + * This processor is expected to be called after {@link ResizingProcessor}. > + */ > +public class MinimumSizeProcessor implements Processor { There are a bunch of unit tests for the icon code. Could we add some for the new functionality too? ::: mobile/android/base/java/org/mozilla/gecko/icons/processing/MinimumSizeProcessor.java:33 (Diff revision 1) > + if (response.getBitmap().getWidth() >= request.getMinimumSizePxAfterScaling()) { > + return; > + } > + > + // This is fragile: ideally, we can return the generated response but instead we're mutating the argument. > + final IconResponse generatedResponse = IconGenerator.generate(request.getContext(), request.getPageUrl()); nit: This is somewhat side-stepping that the "generator" for this execution is defined in the IconTask. But I guess fixing this would require passing the IconTask instance to all processors. Not sure if this refactor is worth it. I'm finding with landing this as is (although writing a test might be a bit cumbersome with the static method call).
Attachment #8906868 -
Flags: review?(s.kaspari) → review+
Assignee | ||
Comment 36•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8906170 [details] Bug 1388396: Add IconRequestBuilder.forActivityStream. https://reviewboard.mozilla.org/r/177906/#review183804 > nit: This is class does not exist yet. :) `ResizingProcessor` is already in the code - `MinimumSizeProcessor` was added later.
Assignee | ||
Comment 37•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8906868 [details] Bug 1388396: Add MinimumSizeProcessor; .forActivityStream takes effect. https://reviewboard.mozilla.org/r/178602/#review183808 > There are a bunch of unit tests for the icon code. Could we add some for the new functionality too? I wrote a unit test as a follow up commit which I will self review - feel free to look into it too. > nit: This is somewhat side-stepping that the "generator" for this execution is defined in the IconTask. But I guess fixing this would require passing the IconTask instance to all processors. Not sure if this refactor is worth it. I'm finding with landing this as is (although writing a test might be a bit cumbersome with the static method call). > nit: This is somewhat side-stepping that the "generator" for this execution is defined in the IconTask. I want to generate an icon, which I feel should conceptually be a static function with no side effects, independent of an object, which `generate` is. Ideally, this function would not be in the IconGenerator class because the naming/relationship implies the `generate` function should rely on the state of an `IconGenerator`. In practice, it feels okay that this is side-stepping the generator of the IconTask because the function is indedpendent. > (although writing a test might be a bit cumbersome with the static method call). The function has no side effects so I don't think we'd need to mock it, just the arguments passed in (specifically the Context since it returns resources). --- I made no changes to my code.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 39•7 years ago
|
||
mozreview-review |
Comment on attachment 8907198 [details] Bug 1388396 - review: Add TestMinimumSizeProcessor. https://reviewboard.mozilla.org/r/178876/#review183958 r=trivial
Attachment #8907198 -
Flags: review?(michael.l.comella) → review+
Comment 40•7 years ago
|
||
Pushed by michael.l.comella@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0aff84a0a8af Increase max favicon scale factor to 3. r=sebastian https://hg.mozilla.org/integration/autoland/rev/8756322ebb4a Add IconRequestBuilder.forActivityStream. r=sebastian https://hg.mozilla.org/integration/autoland/rev/77f982a2f17b Use IconRequestBuilder.forActivityStream in AS UI. r=sebastian https://hg.mozilla.org/integration/autoland/rev/44fa7b379742 Add MinimumSizeProcessor; .forActivityStream takes effect. r=sebastian https://hg.mozilla.org/integration/autoland/rev/3300c15011d3 review: Add TestMinimumSizeProcessor. r=mcomella
Comment 41•7 years ago
|
||
Backed out for failing linter android-test's TestResizingProcessor. testBitmapIsNotScaledMoreThanTwoTimesTheSize: https://hg.mozilla.org/integration/autoland/rev/aa3158628f1a1d974a310477469bdd9359c529fe https://hg.mozilla.org/integration/autoland/rev/540b7549aaa8318b955163249b106ff54e89163a https://hg.mozilla.org/integration/autoland/rev/cae0b4b17d6595646502269ed291126269f0c447 https://hg.mozilla.org/integration/autoland/rev/dec35bc5233c819c7468223e6dbb1e5bdca06697 https://hg.mozilla.org/integration/autoland/rev/760b27ede9924a024c4fb9a8ca5f2ca93704c1ae Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3300c15011d3a6661aaf8eee254b693d7e99f392&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130498968&repo=autoland
Flags: needinfo?(michael.l.comella)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 48•7 years ago
|
||
mozreview-review |
Comment on attachment 8907709 [details] Bug 1388396 - bustage: Fix testBitmapNotScaledMoreThanTwoTimesTheSize. https://reviewboard.mozilla.org/r/179378/#review184540 r=trivial
Attachment #8907709 -
Flags: review?(michael.l.comella) → review+
Comment 49•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 35b990349de9 -d 34b7638b00c8: rebasing 419816:35b990349de9 "Bug 1388396: Increase max favicon scale factor to 3. r=sebastian" merging mobile/android/base/java/org/mozilla/gecko/icons/processing/ResizingProcessor.java rebasing 419817:79d36366ad97 "Bug 1388396: Add IconRequestBuilder.forActivityStream. r=sebastian" merging mobile/android/base/java/org/mozilla/gecko/icons/IconRequest.java merging mobile/android/base/java/org/mozilla/gecko/icons/IconRequestBuilder.java rebasing 419818:bf33800ed618 "Bug 1388396: Use IconRequestBuilder.forActivityStream in AS UI. r=sebastian" merging mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/StreamOverridablePageIconLayout.java merging mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java warning: conflicts while merging mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(michael.l.comella)
Comment 56•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 1f29845030ca -d 0d2d6244f901: rebasing 419860:1f29845030ca "Bug 1388396: Increase max favicon scale factor to 3. r=sebastian" rebasing 419861:187058bad7d6 "Bug 1388396: Add IconRequestBuilder.forActivityStream. r=sebastian" rebasing 419862:488b67ad89f7 "Bug 1388396: Use IconRequestBuilder.forActivityStream in AS UI. r=sebastian" merging mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java warning: conflicts while merging mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Assignee | ||
Comment 57•7 years ago
|
||
Oh, I guess something landed on the autoland tree that I can't see because I'm on mozilla-central. Argh.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 64•7 years ago
|
||
bug 1389296 conflicted with this on autoland so I pulled that down locally, rebased on top of it, pushed my updated commits to reviewboard, and I expect this to now land on autoland.
Depends on: 1389296
Comment 65•7 years ago
|
||
Pushed by michael.l.comella@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ada7d6900274 Increase max favicon scale factor to 3. r=sebastian https://hg.mozilla.org/integration/autoland/rev/a5f78f2e5bfd Add IconRequestBuilder.forActivityStream. r=sebastian https://hg.mozilla.org/integration/autoland/rev/b79ec5330563 Use IconRequestBuilder.forActivityStream in AS UI. r=sebastian https://hg.mozilla.org/integration/autoland/rev/bca0b077f039 Add MinimumSizeProcessor; .forActivityStream takes effect. r=sebastian https://hg.mozilla.org/integration/autoland/rev/84a794ceb84b review: Add TestMinimumSizeProcessor. r=mcomella https://hg.mozilla.org/integration/autoland/rev/77a524d860a0 bustage: Fix testBitmapNotScaledMoreThanTwoTimesTheSize. r=mcomella
Comment 66•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ada7d6900274 https://hg.mozilla.org/mozilla-central/rev/a5f78f2e5bfd https://hg.mozilla.org/mozilla-central/rev/b79ec5330563 https://hg.mozilla.org/mozilla-central/rev/bca0b077f039 https://hg.mozilla.org/mozilla-central/rev/84a794ceb84b https://hg.mozilla.org/mozilla-central/rev/77a524d860a0
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•3 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
•