Summary: | Allow checking whether image was created from JavaScript | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rob Buis <rbuis> | ||||||||
Component: | Page Loading | Assignee: | Rob Buis <rbuis> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | beidson, cdumez, commit-queue, darin, esprehn+autocc, ews-watchlist, gyuyoung.kim, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Other | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 196698 | ||||||||||
Attachments: |
|
Description
Rob Buis
2019-08-13 01:47:56 PDT
Created attachment 376150 [details]
Patch
Created attachment 376195 [details]
Patch
https://bugs.webkit.org/show_bug.cgi?id=200764 is also related, since it should build on this patch. Comment on attachment 376195 [details]
Patch
Normally I suggest that people break down their changes into smaller patches, but in this case it seems strange to land the new HTMLImageElement::createdByParser function without the caller. The implementation looks OK, but I can’t tell the important questions like: If you create a document and parse an image and then change its attributes and move it into another document it seems you can "fake" created by parser. So is that a problem? Depends what we are using this for.
(In reply to Darin Adler from comment #4) > Comment on attachment 376195 [details] > Patch > > Normally I suggest that people break down their changes into smaller > patches, but in this case it seems strange to land the new > HTMLImageElement::createdByParser function without the caller. The > implementation looks OK, but I can’t tell the important questions like: If > you create a document and parse an image and then change its attributes and > move it into another document it seems you can "fake" created by parser. So > is that a problem? Depends what we are using this for. Sorry for the delay, I mailed some folks about this but got no feedback yet. The function is supposed to be used by the lazy image loading logic (https://bugs.webkit.org/show_bug.cgi?id=200764). Basically it gives an easy way to keep legacy eager loading behavior for Images added from JS. This is being discussed in https://github.com/whatwg/html/pull/3752. The latest suggestion is to use "connected" logic instead, however I think that only works when async loading/microtask is implemented, which is not yet the case in WebKit. Your move scenario is a good find. It does seem it is possible that way to fool the system with the patch as-is. A fix could be clearing the flag in didMoveToNewDocument. Since the main object was to keep legacy behavior working, this would not break that, and since it is quite some trouble to fool the system, and lazy image load is new and behind a runtime flag, I think landing this with a FIXME is okay. However, I can also wait a bit longer for feedback. Created attachment 377975 [details]
Patch
After more feedback, it is not entirely sure spec wise to go for the "connected" or "parser-created " concept. I think for now the patch is fine so I'll land. However we will likely need tests so I opened: https://bugs.webkit.org/show_bug.cgi?id=201460 Comment on attachment 377975 [details] Patch Clearing flags on attachment: 377975 Committed r249480: <https://trac.webkit.org/changeset/249480> All reviewed patches have been landed. Closing bug. |