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

RESOLVED FIXED in mozilla1.9.1b1

Status

()

Core
SVG
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: longsonr, Assigned: longsonr)

Tracking

({regression, testcase})

Trunk
mozilla1.9.1b1
regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Created attachment 338529 [details]
reftest
(Assignee)

Comment 1

10 years ago
nsImageLoadingContent::LoadImage now does nothing when presented with the unset URI
Blocks: 444931
Keywords: testcase
(Assignee)

Comment 2

10 years ago
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.
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.
Flags: blocking1.9.1?
s/src=""/xlink:href=""/ of course.
(Assignee)

Comment 5

10 years ago
Created attachment 338623 [details] [diff] [review]
patch

Needed to change from DidChangeString as that is called too early for HasAttr to work correctly.
Assignee: nobody → longsonr
Status: NEW → ASSIGNED
Attachment #338623 - Flags: superreview?(bzbarsky)
Attachment #338623 - Flags: review?(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.
Attachment #338623 - Flags: superreview?(bzbarsky)
Attachment #338623 - Flags: superreview+
Attachment #338623 - Flags: review?(bzbarsky)
Attachment #338623 - Flags: review+
(Assignee)

Comment 7

10 years ago
Created attachment 339258 [details] [diff] [review]
address review comment
[Checkin: Comment 8]
Attachment #338623 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Comment on attachment 339258 [details] [diff] [review]
address review comment
[Checkin: Comment 8]

http://hg.mozilla.org/mozilla-central/rev/72a9124cdb20
Attachment #339258 - Attachment description: address review comment → address review comment [Checkin: Comment 8]
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: blocking1.9.1?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
(Assignee)

Comment 9

10 years ago
Created attachment 341069 [details] [diff] [review]
reftest
(Assignee)

Comment 10

10 years ago
reftest was already checked in as part of bug 448831 and was enabled by 32f14bc9bdda
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.