wallpaper: remove a hack to clear old images
When an img src tag is changed, the old image is visible while the new one loads. There was a previous hacky workaround for wallpaper that manually removed img src tags when navigating away from a wallpaper collection. This hack + switching to cr-auto-img in https://crrev.com/c/3488694 caused a bug where viewing the same wallpaper collection twice in a row incorrectly removed the img src attribute. Instead, build this into cr-auto-img with clear-src attr. When cr-auto-img receives a new auto-src attribute, clear the src first and then set the new src. This prevents the old image from being shown while the new one loads, and removes the need for a wallpaper specific hack. BUG=1304790 BUG=b:222752163 BUG=b:219799872 TEST=open wallpaper picker, navigate through collections, return to same collection after clicking back TEST=browser_tests --gtest_filter="*CrElementsAutoImgTest*" Change-Id: I048951e17af318e986f402ba1f85d1b6c2b7a5e6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3507496 Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org> Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Commit-Queue: Jeffrey Young <cowmoo@chromium.org> Cr-Commit-Position: refs/heads/main@{#979363}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
e8fe77e43e
commit
3773b71f7c
ash/webui/personalization_app/resources/untrusted
chrome/test/data/webui/cr_elements
ui/webui/resources/cr_elements/cr_auto_img
@ -19,7 +19,7 @@
|
||||
<div class$="[[getClassForImagesContainer_(item)]]">
|
||||
<template is="dom-repeat" items="[[item.preview]]" as="preview">
|
||||
<img is="cr-auto-img" auto-src="[[preview.url]]"
|
||||
aria-hidden="true">
|
||||
aria-hidden="true" clear-src>
|
||||
</template>
|
||||
</div>
|
||||
<div class="photo-text-container">
|
||||
@ -37,7 +37,7 @@
|
||||
on-click="onCollectionSelected_" on-keypress="onCollectionSelected_">
|
||||
<div class$="[[getClassForImagesContainer_(item)]]">
|
||||
<img is="cr-auto-img" auto-src="[[getImageUrlForEmptyTile_(item)]]"
|
||||
aria-hidden="true">
|
||||
aria-hidden="true" clear-src>
|
||||
</div>
|
||||
<div class="photo-text-container">
|
||||
<p title$="[[item.name]]">[[item.name]]</p>
|
||||
@ -53,7 +53,7 @@
|
||||
<template is="dom-repeat" items="[[item.preview]]" as="preview">
|
||||
<img is="cr-auto-img" auto-src="[[preview.url]]"
|
||||
aria-hidden="true" on-load="onImgLoad_" on-error="onImgLoad_"
|
||||
with-cookies="[[isGooglePhotosTile_(item)]]">
|
||||
with-cookies="[[isGooglePhotosTile_(item)]]" clear-src>
|
||||
</template>
|
||||
</div>
|
||||
<div class="photo-text-container" hidden>
|
||||
|
@ -25,7 +25,7 @@
|
||||
<div class="photo-images-container">
|
||||
<template is="dom-repeat" items="[[item.preview]]" as="preview">
|
||||
<img is="cr-auto-img" class$="[[getClassForImg_(index, item)]]"
|
||||
auto-src="[[preview.url]]" aria-hidden="true">
|
||||
auto-src="[[preview.url]]" aria-hidden="true" clear-src>
|
||||
</template>
|
||||
<iron-icon icon="personalization:checkmark"></iron-icon>
|
||||
</div>
|
||||
|
@ -2,6 +2,7 @@
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
import '//resources/cr_elements/cr_auto_img/cr_auto_img.js';
|
||||
import '//resources/polymer/v3_0/iron-list/iron-list.js';
|
||||
import './setup.js';
|
||||
import '../trusted/wallpaper/styles.js';
|
||||
@ -72,19 +73,6 @@ export class ImagesGrid extends PolymerElement {
|
||||
if (validateReceivedData(event)) {
|
||||
visible = event.visible;
|
||||
}
|
||||
if (!visible) {
|
||||
// TODO(b/219799872) revisit if this is necessary.
|
||||
// When the grid is hidden, do some dom magic to hide old image
|
||||
// content. This is in preparation for a user switching to a new
|
||||
// wallpaper collection and loading a new set of images.
|
||||
const ironList = this.shadowRoot!.querySelector('iron-list');
|
||||
const images: NodeListOf<HTMLImageElement> =
|
||||
ironList!.querySelectorAll('.photo-container img');
|
||||
for (const image of images) {
|
||||
image.src = '';
|
||||
}
|
||||
this.tiles_ = [];
|
||||
}
|
||||
if (visible) {
|
||||
// If iron-list items were updated while this iron-list was hidden,
|
||||
// the layout will be incorrect. Trigger another layout when iron-list
|
||||
|
@ -8,6 +8,26 @@ import {CrAutoImgElement} from 'chrome://resources/cr_elements/cr_auto_img/cr_au
|
||||
|
||||
import {assertEquals} from 'chrome://webui-test/chai_assert.js';
|
||||
|
||||
async function waitForAttributeChange(
|
||||
element: HTMLElement, attribute: string): Promise<Array<MutationRecord>> {
|
||||
return new Promise(resolve => {
|
||||
const observer = new MutationObserver((mutations, obs) => {
|
||||
obs.disconnect();
|
||||
resolve(mutations);
|
||||
});
|
||||
observer.observe(
|
||||
element,
|
||||
{
|
||||
attributes: true,
|
||||
attributeFilter: [attribute],
|
||||
attributeOldValue: true,
|
||||
childList: false,
|
||||
subtree: false,
|
||||
},
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
suite('CrAutoImgElementTest', () => {
|
||||
let img: CrAutoImgElement;
|
||||
|
||||
@ -77,4 +97,30 @@ suite('CrAutoImgElementTest', () => {
|
||||
encodeURIComponent(autoSrc)}&withCookies=true`,
|
||||
img.src);
|
||||
});
|
||||
|
||||
test(
|
||||
'setting clear-src removes the src attribute first when auto-src changes',
|
||||
async () => {
|
||||
const originalSrc = 'chrome://foo/foo.png';
|
||||
img.clearSrc = '';
|
||||
img.autoSrc = originalSrc;
|
||||
assertEquals(
|
||||
originalSrc, img.src, 'src attribute is set to initial value');
|
||||
|
||||
const newSrc = 'chrome://bar/bar.png';
|
||||
|
||||
const attrChangedPromise = waitForAttributeChange(img, 'src');
|
||||
img.autoSrc = newSrc;
|
||||
|
||||
const mutations = await attrChangedPromise;
|
||||
assertEquals(2, mutations.length, 'src is changed twice');
|
||||
assertEquals(
|
||||
originalSrc, mutations[0]?.oldValue,
|
||||
'src starts as original value');
|
||||
|
||||
assertEquals(
|
||||
null, mutations[1]?.oldValue, 'src is set to null in between');
|
||||
|
||||
assertEquals(newSrc, img.src, 'src attribute is set to new value');
|
||||
});
|
||||
});
|
||||
|
@ -13,13 +13,17 @@
|
||||
*
|
||||
* 2. In HTML instantiate
|
||||
*
|
||||
* <img is="cr-auto-img" auto-src="https://foo.com/bar.png"></img>
|
||||
* <img is="cr-auto-img" auto-src="https://foo.com/bar.png">
|
||||
*
|
||||
* If your image needs to be fetched using cookies, you can use the
|
||||
* with-cookies attribute as follows:
|
||||
*
|
||||
* <img is="cr-auto-img" auto-src="https://foo.com/bar.png" with-cookies>
|
||||
* </img>
|
||||
*
|
||||
* If you want the image to reset to an empty state when auto-src changes
|
||||
* and the new image is still loading, set the clear-src attribute:
|
||||
*
|
||||
* <img is="cr-auto-img" auto-src="[[calculateSrc()]]" clear-src>
|
||||
*
|
||||
* NOTE: Since <cr-auto-img> may use the chrome://image data source some images
|
||||
* may be transcoded to PNG.
|
||||
@ -28,6 +32,9 @@
|
||||
/** @type {string} */
|
||||
const AUTO_SRC = 'auto-src';
|
||||
|
||||
/** @type {string} */
|
||||
const CLEAR_SRC = 'clear-src';
|
||||
|
||||
/** @type {string} */
|
||||
const WITH_COOKIES = 'with-cookies';
|
||||
|
||||
@ -41,6 +48,12 @@ export class CrAutoImgElement extends HTMLImageElement {
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.hasAttribute(CLEAR_SRC)) {
|
||||
// Remove the src attribute so that the old image is not shown while the
|
||||
// new one is loading.
|
||||
this.removeAttribute('src');
|
||||
}
|
||||
|
||||
let url = null;
|
||||
try {
|
||||
url = new URL(newValue || '');
|
||||
@ -72,6 +85,16 @@ export class CrAutoImgElement extends HTMLImageElement {
|
||||
return this.getAttribute(AUTO_SRC);
|
||||
}
|
||||
|
||||
/** @param {string} _ */
|
||||
set clearSrc(_) {
|
||||
this.setAttribute(CLEAR_SRC, '');
|
||||
}
|
||||
|
||||
/** @return {string} */
|
||||
get clearSrc() {
|
||||
return this.getAttribute(CLEAR_SRC);
|
||||
}
|
||||
|
||||
/** @param {string} _ */
|
||||
set withCookies(_) {
|
||||
this.setAttribute(WITH_COOKIES, '');
|
||||
|
Reference in New Issue
Block a user