block.html - Issue 5121634776121344: Issue 2049 - Wrong logo being displayed in Options and Block Element dialog

Delta Between Two Patch Sets: block.html

Issue 5121634776121344: Issue 2049 - Wrong logo being displayed in Options and Block Element dialog (Closed)

Left Patch Set: Created Feb. 26, 2015, 9 p.m.

Right Patch Set: Removed redundant descriptors Created Feb. 27, 2015, 6:45 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2	2

3 <!--	3 <!--

4 - This file is part of Adblock Plus <https://adblockplus.org/>,	4 - This file is part of Adblock Plus <https://adblockplus.org/>,

5 - Copyright (C) 2006-2015 Eyeo GmbH	5 - Copyright (C) 2006-2015 Eyeo GmbH

6 -	6 -

7 - Adblock Plus is free software: you can redistribute it and/or modify	7 - Adblock Plus is free software: you can redistribute it and/or modify

8 - it under the terms of the GNU General Public License version 3 as	8 - it under the terms of the GNU General Public License version 3 as

9 - published by the Free Software Foundation.	9 - published by the Free Software Foundation.

10 -	10 -

(...skipping 54 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
65 }	65 }

66 button	66 button

67 {	67 {

68 padding: 3px !important;	68 padding: 3px !important;

69 }	69 }

70 </style>	70 </style>

71 </head>	71 </head>

72 <body draggable="true">	72 <body draggable="true">

73	73

74 <div id="title">	74 <div id="title">

75 <img src="icons/detailed/abp-32.png" srcset="icons/detailed/abp-32.png 1x, ic ons/detailed/abp-64.png 2x">	75 <img src="icons/detailed/abp-32.png" srcset="icons/detailed/abp-64.png 2x">
Sebastian Noack 2015/02/27 12:49:42 Nit: redundant space before "srcset". Nit: redundant space before "srcset". Sebastian Noack 2015/02/27 12:49:42 Nit: You don't need to redundantly specify the org Nit: You don't need to redundantly specify the orginal image in the "srcset". If there is no image for "1x", it falls back to the "src" attribute anyway. Sebastian Noack 2015/02/27 12:49:42 How about specifying the abp-48.png for 1.5x? How about specifying the abp-48.png for 1.5x? Wladimir Palant 2015/02/27 18:46:20 I verified that this indeed matches the current im Show quoted text On 2015/02/27 12:49:42, Sebastian Noack wrote: > Nit: You don't need to redundantly specify the orginal image in the "srcset". If > there is no image for "1x", it falls back to the "src" attribute anyway. I verified that this indeed matches the current implementation - MDN documentation contradicted the spec here. Apparently, the MDN documentation was wrong so I fixed it. Show quoted text > How about specifying the abp-48.png for 1.5x? For what purpose? There are no screens with 1.5x pixel density, and we definitely won't provide images for this intermediate size. Sebastian Noack 2015/02/27 18:53:12 This isn't about screens, but about operating syst Show quoted text On 2015/02/27 18:46:20, Wladimir Palant wrote: > On 2015/02/27 12:49:42, Sebastian Noack wrote: > > Nit: You don't need to redundantly specify the orginal image in the "srcset". > If > > there is no image for "1x", it falls back to the "src" attribute anyway. > > I verified that this indeed matches the current implementation - MDN > documentation contradicted the spec here. Apparently, the MDN documentation was > wrong so I fixed it. > > > How about specifying the abp-48.png for 1.5x? > > For what purpose? There are no screens with 1.5x pixel density, and we > definitely won't provide images for this intermediate size. This isn't about screens, but about operating system configuration. It's merely coincidentally that OS X automatically set's pixel density to 2x on devices with Retina display. However, on Windows 8 for instance, you have more fine-grained options, and a pixel density of 1.5x isn't entirely uncommon there. I checked how Wikipedia is doing it.You might want to inspect the example image on https://en.wikipedia.org/wiki/Portable_Network_Graphics to get an idea. Wladimir Palant 2015/02/27 19:04:30 The point is: 1.5x is an uncommon setting, there a Show quoted text On 2015/02/27 18:53:12, Sebastian Noack wrote: > This isn't about screens, but about operating system configuration. It's merely > coincidentally that OS X automatically set's pixel density to 2x on devices with > Retina display. > > However, on Windows 8 for instance, you have more fine-grained options, and a > pixel density of 1.5x isn't entirely uncommon there. The point is: 1.5x is an uncommon setting, there are no devices that ship with it. Only users who set it manually get that, and that is rare. So we will definitely accept the browser scaling down the larger image here instead of providing yet another set of images. Show quoted text > I checked how Wikipedia is doing it.You might want to inspect the example image > on https://en.wikipedia.org/wiki/Portable_Network_Graphics to get an idea. Please tell me what I am seeing there. From where I am standing, Wikipedia took a 560px image and specified it as both the 1x and 2x URL. The width of the <img> tag is 280px. So Retina users will get the expected result, whereas for users with normal pixel density the browser will scale down the image - like it would do if only the src tag were specified. Frankly, I don't get why Wikipedia is doing such nonsense for an image that they only have in one size. Wladimir Palant 2015/02/27 19:11:17 Ok, it seems that Wikipedia will produce a differe Ok, it seems that Wikipedia will produce a different srcset attribute for Chrome, one that makes more sense. However, in their case it's all the same base image, it has merely been scaled down automatically on the server. The browser could do the scaling just as well - so the only reason they must be doing that is giving people smaller downloads. In our case, providing separate images for 1.5x pixel density would actually produce the exact opposite result and increase the download size. Sebastian Noack 2015/02/27 19:16:01 Sure 2x is way more common. But if we have a match Show quoted text On 2015/02/27 19:04:30, Wladimir Palant wrote: > On 2015/02/27 18:53:12, Sebastian Noack wrote: > > This isn't about screens, but about operating system configuration. It's > merely > > coincidentally that OS X automatically set's pixel density to 2x on devices > with > > Retina display. > > > > However, on Windows 8 for instance, you have more fine-grained options, and a > > pixel density of 1.5x isn't entirely uncommon there. > > The point is: 1.5x is an uncommon setting, there are no devices that ship with > it. Only users who set it manually get that, and that is rare. So we will > definitely accept the browser scaling down the larger image here instead of > providing yet another set of images. Sure 2x is way more common. But if we have a matching icon for 1.5x in our build anyway why not using it? Show quoted text > > I checked how Wikipedia is doing it.You might want to inspect the example > image > > on https://en.wikipedia.org/wiki/Portable_Network_Graphics to get an idea. > > Please tell me what I am seeing there. From where I am standing, Wikipedia took > a 560px image and specified it as both the 1x and 2x URL. The width of the <img> > tag is 280px. So Retina users will get the expected result, whereas for users > with normal pixel density the browser will scale down the image - like it would > do if only the src tag were specified. Frankly, I don't get why Wikipedia is > doing such nonsense for an image that they only have in one size. This were common practice to support Retina displays, before srcset were introduced. However, apparently Wikipedia is using a different set of images depending on the browser. On Chrome they specify a "src" attribute with the original image size and a srcset with one image for 1.5x and another one for 2x, just as I suggested to do here. Sebastian Noack 2015/02/27 19:18:37 Are you talking about the few bytes we would add b Show quoted text On 2015/02/27 19:11:17, Wladimir Palant wrote: > Ok, it seems that Wikipedia will produce a different srcset attribute for > Chrome, one that makes more sense. However, in their case it's all the same base > image, it has merely been scaled down automatically on the server. The browser > could do the scaling just as well - so the only reason they must be doing that > is giving people smaller downloads. In our case, providing separate images for > 1.5x pixel density would actually produce the exact opposite result and increase > the download size. Are you talking about the few bytes we would add by configuring an additional image here? As I said, I'm not talking about adding a new image to the build, but merely about using those we already have in the build. Wladimir Palant 2015/02/27 19:26:59 If we support a particular pixel density, we shoul Show quoted text On 2015/02/27 19:18:37, Sebastian Noack wrote: > Are you talking about the few bytes we would add by configuring an additional > image here? As I said, I'm not talking about adding a new image to the build, > but merely about using those we already have in the build. If we support a particular pixel density, we should do it consistently. Right now we are starting to support 2x pixel density properly. We still need to add additional versions of all images, e.g. of the social icons. It isn't going to end with our logo. And - no, just because we happen to have one image of the right size already for 1.5x pixel density (on Chrome only btw., it's not part of the Safari build right now) we shouldn't use it. It should be consistent. Sebastian Noack 2015/02/27 19:29:56 Fair enough. Show quoted text On 2015/02/27 19:26:59, Wladimir Palant wrote: > On 2015/02/27 19:18:37, Sebastian Noack wrote: > > Are you talking about the few bytes we would add by configuring an additional > > image here? As I said, I'm not talking about adding a new image to the build, > > but merely about using those we already have in the build. > > If we support a particular pixel density, we should do it consistently. Right > now we are starting to support 2x pixel density properly. We still need to add > additional versions of all images, e.g. of the social icons. It isn't going to > end with our logo. > > And - no, just because we happen to have one image of the right size already for > 1.5x pixel density (on Chrome only btw., it's not part of the Safari build right > now) we shouldn't use it. It should be consistent. Fair enough.
76 <span class="i18n_add_filters_msg"></span>	76 <span class="i18n_add_filters_msg"></span>

77 </div>	77 </div>

78	78

79 <textarea id="filters"></textarea>	79 <textarea id="filters"></textarea>

80	80

81 <div id="buttons">	81 <div id="buttons">

82 <button id="addButton" class="i18n_add"></button>	82 <button id="addButton" class="i18n_add"></button>

83 <button id="cancelButton" class="i18n_cancel"></button>	83 <button id="cancelButton" class="i18n_cancel"></button>

84 </div>	84 </div>

85	85

86 </body>	86 </body>

87 </html>	87 </html>

LEFT	RIGHT