Closed Bug 1370705 Opened 3 years ago Closed 3 years ago

Move side effects of setting attribute of HTMLImageElement to after the attribute is set

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: bytesized, Assigned: bytesized)

References

Details

Attachments

(1 file)

The side effects [1] of setting attributes of HTMLImageElement should really happen after the attribute is set rather than before. This will be more important when Bug 1365092 lands, as then moving those side effects should allow the removal of the member variable HTMLImageElement::mForceReload which will be added in that patch.

[1] http://searchfox.org/mozilla-central/rev/20963d7269b1b14d455f47bc0260d0653015bf84/dom/html/HTMLImageElement.cpp#525-580
Priority: -- → P3
Assignee: nobody → ksteuber
Attachment #8875920 - Flags: review?(bzbarsky)
Comment on attachment 8875920 [details]
Bug 1370705 - Move attribute change effects from HTMLImageElement::BeforeMaybeChangeAttr to HTMLImageElement::AfterMaybeChangeAttr

https://reviewboard.mozilla.org/r/147324/#review152168

r=me

::: dom/html/HTMLImageElement.cpp:514
(Diff revision 3)
>        mNewRequestsWillNeedAnimationReset = true;
>  
>        // Force image loading here, so that we'll try to load the image from
>        // network if it's set to be not cacheable...  If we change things so that
>        // the state gets in Element's attr-setting happen around this
>        // LoadImage call, we could start passing false instead of aNotify

I think in the value changed case this is already the case.  But in the "same value" case it might not be... the comment might need updating accordingly.
Attachment #8875920 - Flags: review?(bzbarsky) → review+
Pushed by ksteuber@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0b1ae7752ab4
Move attribute change effects from HTMLImageElement::BeforeMaybeChangeAttr to HTMLImageElement::AfterMaybeChangeAttr r=bz
https://hg.mozilla.org/mozilla-central/rev/0b1ae7752ab4
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.