455226 - Removing the xlink:href attribute of an feImage filter does not cause it to change

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert Longson [:longsonr]]

Attachment: reftest

Comment 1

[Robert Longson [:longsonr]]

nsImageLoadingContent::LoadImage now does nothing when presented with the unset URI

Comment 2

[Robert Longson [:longsonr]]

I imagine you can get the same failure to refresh when removing the xlink:href attribute of an ordinary SVG image element or the src attribute of an HTML image element.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Removing the src attribute of an HTML image element never "refreshed" (for compat reasons).  See lack of UnsetAttr override.

Note that loading "" is the wrong thing to do when the src is unset; the right thing to do is to drop the existing image, not do a bogus load, unless you mean to tell me that <svg:image/> and <svg:image src=""/> should have identical behavior (I'm not buying it).

So sounds like we need the following changes:

1)  Fix the SVG code to not do bogus loads.
2)  If SVG expects href="" to work, add a boolean to LoadImage() that would
    allow callers to force it to work.

Comment 4

[Boris Zbarsky [:bzbarsky]]

s/src=""/xlink:href=""/ of course.

Comment 5

[Robert Longson [:longsonr]]

Attachment: patch

Needed to change from DidChangeString as that is called too early for HasAttr to work correctly.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 338623 [details] [diff] [review]
patch

>+++ b/content/svg/content/src/nsSVGFilters.cpp
>+nsSVGFEImageElement::AfterSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
>+    if (HasAttr(kNameSpaceID_XLink, nsGkAtoms::href)) {

  if (aValue) {

seems faster, no?

Same thing in nsSVGImageElement, and looks good.

Comment 7

[Robert Longson [:longsonr]]

Attachment: address review comment [Checkin: Comment 8]

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 339258 [details] [diff] [review]
address review comment
[Checkin: Comment 8]

http://hg.mozilla.org/mozilla-central/rev/72a9124cdb20

Comment 9

[Robert Longson [:longsonr]]

Attachment: reftest

Comment 10

[Robert Longson [:longsonr]]

reftest was already checked in as part of bug 448831 and was enabled by 32f14bc9bdda