Mike, I suspect this is regression from your changes in bug 1058909. Could you have a look?

Thanks for filing, I also noticed this.

(In reply to Lucas Rocha (:lucasr) from comment #0) > Mike, I suspect this is regression from your changes in bug 1058909. Backed out bug 1058909 locally and the issue still remains.

Regression window wanted - at Nightly increments should be fine.

Regression window: mozilla central: good build: 25-09 bad build: 26-09 pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1735ff2bb23e&tochange=9e3d649b80a2 inbound: good build: 1411644116 bad build: 1411649004 pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e2c803c2aeec&tochange=128d8d99b72d

Mike, this is what caused the regression: https://hg.mozilla.org/mozilla-central/rev/540bc6af7020 More specifically, you removed the 4dp left padding from the favicon view (in toolbar_display_layout.xml) which ends up giving it more space for the image.

(In reply to Lucas Rocha (:lucasr) from comment #6) > Mike, this is what caused the regression: Yep, seems you are correct - I don't think I noticed the difference in size when testing. x_x Here's a patch, however, ideally we'd use margins to offset the favicon so that the favicon size we specify is exact and when we put it into memory, we don't have to store larger favicons than necessary. We can't, however, because margins are not part of the hit area of a button (at least in my experiences, at least on older devices). To avoid code duplication, I don't think it's worth having two resource values, however. We could add a Favicons.getFaviconSize method, but that seems messy considering we'd need to pull in the layout to get and subtract away padding. Is this the optimal change, Lucas?

(In reply to Michael Comella (:mcomella) from comment #8) > (In reply to Lucas Rocha (:lucasr) from comment #6) > > Mike, this is what caused the regression: > > Yep, seems you are correct - I don't think I noticed the difference in size > when testing. x_x > > Here's a patch, however, ideally we'd use margins to offset the favicon so > that the favicon size we specify is exact and when we put it into memory, we > don't have to store larger favicons than necessary. We can't, however, > because margins are not part of the hit area of a button (at least in my > experiences, at least on older devices). > > To avoid code duplication, I don't think it's worth having two resource > values, however. We could add a Favicons.getFaviconSize method, but that > seems messy considering we'd need to pull in the layout to get and subtract > away padding. > > Is this the optimal change, Lucas? Not optimal but good enough. We'll need to revise the toolbar paddings/margins to ensure consistent hit areas throughout. File a follow-up?

FWIW, the reason I'm not too concerned about hit areas here is that the actual hit area is favicon+lock icon on phones.