Bug 19472 – test for dynamic changes to the "data" attribute of an OBJECT element fails

• Component: DOM
• Status: RESOLVED CONFIGURATION CHANGED

[Calvin Anderson]

I Steps:
 Go to http://www.hixie.ch/tests/adhoc/dom/html/flow/object/001.html
 
 II Issue:
 Test fails for dynamic changes to the "data" attribute of an OBJECT element.
 
 III Other browsers:
 IE7: not ok
 FF3: ok
 Opera: ok
 
 IV Nightly tested: 34380

[Mark Rowe (bdash)]

<rdar://problem/5999635>

[Robert Blaut]

The test works correctly if it's loaded on background tab.

Steps to reproduce:
1) Uncheck in Preferences, Tabs option: "Select tabs and windows as they are created"
2) Middle click on URL: http://www.hixie.ch/tests/adhoc/dom/html/flow/object/001.html
3) Test reports "PASSED".

I can confirm the problem.

[Christopher Vickery]

Is there a workaround for this, or is AJAX/innerHTML the only solution?

[Sameer Patil]

Created attachment 111930 [details]
Proposed patch

Even if setUpdateWidget() set to true in HTMLObjectElement after a data attribute change, scheduling a call to updateWidget() is required to update the element. Same is true for HTMLEmbed element if src attribute changes dynamically.

[Sameer Patil]

Created attachment 111931 [details]
Proposed patch

Please ignore previous patch. Forgot to add some test files.

[Sameer Patil]

Created attachment 111948 [details]
Proposed patch

[Sameer Patil]

Can anyone please review my patch.

[Eric Seidel (no email)]

Comment on attachment 111948 [details]
Proposed patch

OK.  I'm glad we have this fix, but why do we need to use a custom timer to make the widget update?

[Darin Adler]

Comment on attachment 111948 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=111948&action=review

> Source/WebCore/html/HTMLPlugInImageElement.cpp:203
> +void HTMLPlugInImageElement::updateWidgetTimerFired(Timer<HTMLPlugInImageElement> *)

Incorrect formatting here. Space before * should be omitted.

> Source/WebCore/html/HTMLPlugInImageElement.cpp:205
> +    m_updateWidgetTimer.stop();

No need to stop the timer; it’s a one-shot.

> Source/WebCore/html/HTMLPlugInImageElement.h:61
> +    Timer<HTMLPlugInImageElement> m_updateWidgetTimer;

A timer is a large object. This means we’re consuming a lot more memory. Is there a way to do this without the timer?

[Eric Seidel (no email)]

Comment on attachment 111948 [details]
Proposed patch

r- based on Darin and my comments.  Just need some justification about the timer or thoughts on how we'd do this without one.

[Sameer Patil]

(In reply to comment #10)
> (From update of attachment 111948 [details])
> r- based on Darin and my comments.  Just need some justification about the timer or thoughts on how we'd do this without one.
I had tried this doing without timer by calling updateWidgetIfNecessary()(making it protected in base class) directly from parseMappedAttribute() of object and embed element.

But the main issue here is from updateWidget() call go to SubframeLoader::loadOrRedirectSubframe() in this function instead of loadSubframe(), scheduleLocationChange() gets called. I guess loadSubFrame() needs to be get called to load new data src.

The reason looks likes updateWidget() come from the same stack of previous subframeload(original data src url).

I used timer to accomplish this so previous subframe load will complete first and next load will come on new stack and it will load it as subframe.

Please correct me if my understanding is wrong.

[Darin Adler]

(In reply to comment #11)
> But the main issue here is from updateWidget() call go to SubframeLoader::loadOrRedirectSubframe() in this function instead of loadSubframe(), scheduleLocationChange() gets called. I guess loadSubFrame() needs to be get called to load new data src.

We need to find a clear way to say this in a comment in the code. The answer to “why is a timer needed” should be something stated rather than something people just have to figure out.

[Ahmad Saleem]

On attached test case, all browser (Safari 15.6, Safari Technology Preview 151, Chrome Canary 105 and Firefox Nightly 105) show "PASS" in white text within "Green" rectangle. Is something else needed here? 

I think it is something fixed along the way and this can be marked as "RESOLVED CONFIGURATION CHANGED"? Please ignore myself, if you think the test case needs to be updated and I am testing wrong but just wanted to share updated results. Thanks!