Input: Load <input type=image> image even before LayoutObject created. [chromium/src : main]
Vladimir Levin (Gerrit)
Attention is currently required from: Chris Harrelson, Joey Arhar.
Vladimir Levin would like Chris Harrelson to review this change.
Input: Load <input type=image> image even before LayoutObject created.
This patch removes an early out for no-layout object when processing
a src change. We may not have a layout object because of c-v, but we
should still load the image and fire the load event.
R=jar...@chromium.org
Fixed: 1247844
Change-Id: I6a3c102be669c5d821da87caab613d89145d2c05
---
M third_party/blink/renderer/core/html/forms/image_input_type.cc
M third_party/blink/web_tests/TestExpectations
2 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/third_party/blink/renderer/core/html/forms/image_input_type.cc b/third_party/blink/renderer/core/html/forms/image_input_type.cc
index 51a2518..ae647bd 100644
--- a/third_party/blink/renderer/core/html/forms/image_input_type.cc
+++ b/third_party/blink/renderer/core/html/forms/image_input_type.cc
@@ -130,8 +130,6 @@
}
void ImageInputType::SrcAttributeChanged() {
- if (!GetElement().GetLayoutObject())
- return;
GetElement().EnsureImageLoader().UpdateFromElement(
ImageLoader::kUpdateIgnorePreviousError);
}
diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations
index 4b817c6..92714e7 100644
--- a/third_party/blink/web_tests/TestExpectations
+++ b/third_party/blink/web_tests/TestExpectations
@@ -4678,8 +4678,6 @@
# Temporarily disabled to unblock https://crrev.com/c/2979697
crbug.com/1222114 http/tests/devtools/console/console-dir.js [ Failure Pass ]
-crbug.com/1247844 external/wpt/css/css-contain/content-visibility/content-visibility-input-image.html [ Crash Failure Pass Timeout ]
-
# Expected to fail with SuppressDifferentOriginSubframeJSDialogs feature disabled, tested in VirtualTestSuite
crbug.com/1065085 external/wpt/html/webappapis/user-prompts/cannot-show-simple-dialogs/confirm-different-origin-frame.sub.html [ Failure ]
crbug.com/1065085 external/wpt/html/webappapis/user-prompts/cannot-show-simple-dialogs/prompt-different-origin-frame.sub.html [ Failure ]
To view, visit change 4355259. To unsubscribe, or for help writing mail filters, visit settings.
Vladimir Levin (Gerrit)
Attention is currently required from: Chris Harrelson, Joey Arhar.
1 comment:
Patchset:
Please take a look. It seems the bots are happy with this change. I'm not sure if you know of any negative implications here. We're basically potentially starting the image load before we create a layout object. We need this for input to fire onload in a c-v hidden subtree, but it of course affects all cases where we don't have a layout object.
To view, visit change 4355259. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Attention is currently required from: Chris Harrelson, Vladimir Levin.
1 comment:
Patchset:
Please take a look. It seems the bots are happy with this change. […]
Thanks for working on this!
Does this affect display:none? I don't know what the behavior is, but if it does then maybe it shouldn't...
To view, visit change 4355259. To unsubscribe, or for help writing mail filters, visit settings.
Vladimir Levin (Gerrit)
Attention is currently required from: Chris Harrelson, Joey Arhar.
1 comment:
Patchset:
Thanks for working on this! […]
It definitely does affect display: none behavior, which is why I ran the bots to see if there are any problems. My concern is that <img src=...> still fires onload under display none. In fact, https://html.spec.whatwg.org/multipage/input.html#image-button-state-(type=image) doesn't really talk about style or rendering or layout objects. It only alludes that this step can be skipped if "the user agent only fetches images on demand".
However, if that's the case, then maybe the bug is WontFix, because we don't have to fetch the image in c-v hidden :)
What I'm trying to say is that we should either align <img> with <input type=image> or align behavior of c-v: hidden with behavior of display: none when it comes to <input type=image> and those two approaches are either this patch, or no patch. WDYT?
To view, visit change 4355259. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Attention is currently required from: Chris Harrelson, Vladimir Levin.
1 comment:
Patchset:
It definitely does affect display: none behavior, which is why I ran the bots to see if there are an […]
Yeah I guess input type=image is probably not very important compared to img elements and we can follow the same behavior. Want to just put it behind a RuntimeEnabledFeature in case it blows up the internet?
To view, visit change 4355259. To unsubscribe, or for help writing mail filters, visit settings.
Vladimir Levin (Gerrit)
Attention is currently required from: Chris Harrelson, Joey Arhar.
2 comments:
Patchset:
Yeah I guess input type=image is probably not very important compared to img elements and we can fol […]
Good idea! Done.
Patchset:
PTAL
To view, visit change 4355259. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Attention is currently required from: Chris Harrelson, Vladimir Levin.
Patch set 3:Code-Review +1
Vladimir Levin (Gerrit)
Attention is currently required from: Chris Harrelson.
Patch set 3:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
Input: Load <input type=image> image even before LayoutObject created.
This patch removes an early out for no-layout object when processing
a src change. We may not have a layout object because of c-v, but we
should still load the image and fire the load event.
R=jar...@chromium.org
Fixed: 1247844
Change-Id: I6a3c102be669c5d821da87caab613d89145d2c05
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4355259
Reviewed-by: Joey Arhar <jar...@chromium.org>
Commit-Queue: Vladimir Levin <vmp...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1120142}
---
M third_party/blink/renderer/core/html/forms/image_input_type.cc
M third_party/blink/renderer/platform/runtime_enabled_features.json5
M third_party/blink/web_tests/TestExpectations
3 files changed, 11 insertions(+), 3 deletions(-)