Issue metadata
Sign in to add a comment
|
Twitter favicon fails to display when loaded as an image |
||||||||||||||||||||||
Issue descriptionChrome Version: v70 OS: Win10, Mac, Linux What steps will reproduce the problem? (1) Load https://abs.twimg.com/favicons/favicon.ico in a tab (2) (3) What is the expected result? The Twitter 32x32 blue bird logo should be displayed in the middle of the page What happens instead? There is no image displayed, it seems to be a blank image, having the same color as the background. Additional info: The Vivaldi browser uses a GUI rendered using HTML and JS, this means that favicons are displayed using image elements. This problem therefore breaks the display of (at least) Twitter's favicon in Vivaldi's tab (other favicons do display, e.g. for this site). Chromium do display the icon in its tabs, so decoding apparently works fine, but displaying the icon as a document fails. This is a regression, as it worked in Chromium 69, and until v70.0.3533.0. The regression point appears to be the commit of https://chromium-review.googlesource.com/1185629 Initial debugging have not been able to determine what is causing the problem, the ICO decoder is created, and the decoding functions return successfully.
,
Sep 14
https://chromium-review.googlesource.com/c/chromium/src/+/1227397, fix up. This will need a merge to 70.
,
Sep 14
Thanks, have tested it on our branch and seems to work fine.
,
Sep 14
,
Sep 17
,
Sep 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e04b4da70bce0990f7a9df5f92edf129b393ef4 commit 9e04b4da70bce0990f7a9df5f92edf129b393ef4 Author: Khushal <khushalsagar@chromium.org> Date: Tue Sep 18 21:26:56 2018 blink/decoders: Make sure ICOImageDecoder decodes to external memory. The ImageDecoder should decode to external memory if provided an allocator. The ICOImageDecoder was missing forwarding this allocator to a png decoder it uses internally. R=chrishtr@chromium.org Bug: 884136 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Iacad54e2640bc4f91d48e9a3c28bc6073b95ba6a Reviewed-on: https://chromium-review.googlesource.com/1227397 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/master@{#592198} [add] https://crrev.com/9e04b4da70bce0990f7a9df5f92edf129b393ef4/third_party/WebKit/LayoutTests/images/png-inside-ico-expected.png [add] https://crrev.com/9e04b4da70bce0990f7a9df5f92edf129b393ef4/third_party/WebKit/LayoutTests/images/png-inside-ico-expected.txt [add] https://crrev.com/9e04b4da70bce0990f7a9df5f92edf129b393ef4/third_party/WebKit/LayoutTests/images/png-inside-ico.html [add] https://crrev.com/9e04b4da70bce0990f7a9df5f92edf129b393ef4/third_party/WebKit/LayoutTests/images/resources/twitter_favicon.ico [add] https://crrev.com/9e04b4da70bce0990f7a9df5f92edf129b393ef4/third_party/WebKit/LayoutTests/virtual/exotic-color-space/images/png-inside-ico-expected.png [modify] https://crrev.com/9e04b4da70bce0990f7a9df5f92edf129b393ef4/third_party/blink/renderer/platform/graphics/image_decoder_wrapper.cc [modify] https://crrev.com/9e04b4da70bce0990f7a9df5f92edf129b393ef4/third_party/blink/renderer/platform/image-decoders/ico/ico_image_decoder.cc
,
Sep 18
I'll request a merge after tomorrow's canary.
,
Sep 19
The NextAction date has arrived: 2018-09-19
,
Sep 19
Verified on latest canary. Merge request for M70.
,
Sep 19
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 20
,
Sep 20
Merge approved for 70, branch 3538.
,
Sep 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b2fa2393c3769b60978517ceb863704cba3903e commit 6b2fa2393c3769b60978517ceb863704cba3903e Author: Khushal <khushalsagar@chromium.org> Date: Thu Sep 20 18:13:23 2018 blink/decoders: Make sure ICOImageDecoder decodes to external memory. The ImageDecoder should decode to external memory if provided an allocator. The ICOImageDecoder was missing forwarding this allocator to a png decoder it uses internally. R=chrishtr@chromium.org Bug: 884136 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Iacad54e2640bc4f91d48e9a3c28bc6073b95ba6a Reviewed-on: https://chromium-review.googlesource.com/1227397 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Khushal <khushalsagar@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#592198}(cherry picked from commit 9e04b4da70bce0990f7a9df5f92edf129b393ef4) Reviewed-on: https://chromium-review.googlesource.com/1236921 Reviewed-by: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#547} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [add] https://crrev.com/6b2fa2393c3769b60978517ceb863704cba3903e/third_party/WebKit/LayoutTests/images/png-inside-ico-expected.png [add] https://crrev.com/6b2fa2393c3769b60978517ceb863704cba3903e/third_party/WebKit/LayoutTests/images/png-inside-ico-expected.txt [add] https://crrev.com/6b2fa2393c3769b60978517ceb863704cba3903e/third_party/WebKit/LayoutTests/images/png-inside-ico.html [add] https://crrev.com/6b2fa2393c3769b60978517ceb863704cba3903e/third_party/WebKit/LayoutTests/images/resources/twitter_favicon.ico [add] https://crrev.com/6b2fa2393c3769b60978517ceb863704cba3903e/third_party/WebKit/LayoutTests/virtual/exotic-color-space/images/png-inside-ico-expected.png [modify] https://crrev.com/6b2fa2393c3769b60978517ceb863704cba3903e/third_party/blink/renderer/platform/graphics/image_decoder_wrapper.cc [modify] https://crrev.com/6b2fa2393c3769b60978517ceb863704cba3903e/third_party/blink/renderer/platform/image-decoders/ico/ico_image_decoder.cc
,
Sep 20
,
Sep 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b2fa2393c3769b60978517ceb863704cba3903e Commit: 6b2fa2393c3769b60978517ceb863704cba3903e Author: khushalsagar@chromium.org Commiter: khushalsagar@chromium.org Date: 2018-09-20 18:13:23 +0000 UTC blink/decoders: Make sure ICOImageDecoder decodes to external memory. The ImageDecoder should decode to external memory if provided an allocator. The ICOImageDecoder was missing forwarding this allocator to a png decoder it uses internally. R=chrishtr@chromium.org Bug: 884136 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Iacad54e2640bc4f91d48e9a3c28bc6073b95ba6a Reviewed-on: https://chromium-review.googlesource.com/1227397 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Khushal <khushalsagar@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#592198}(cherry picked from commit 9e04b4da70bce0990f7a9df5f92edf129b393ef4) Reviewed-on: https://chromium-review.googlesource.com/1236921 Reviewed-by: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#547} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
,
Sep 25
This issue is now not reproducible, works as per expected result on latest M70-70.0.3538.32 and on M71-71.0.3561.0, verified on Samsung Galaxy Tab S2(SM-T815Y) / NRD90M
,
Sep 25
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by schenney@chromium.org
, Sep 14Status: Assigned (was: Untriaged)