xul:image with a SVG filter won't display on "about:" pages

VERIFIED FIXED in mozilla2.0b9

Status

()

defect
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: Unfocused, Assigned: dholbert)

Tracking

Trunk
mozilla2.0b9
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

I have a xul:image element, and in CSS I'm attaching a SVG filter (its a greyscale filter) from a separate file. With the filter the image won't display - it displays fine without the filter.

The same code worked a week or two ago (I can't remember when I previously pulled from mozilla-central). I pulled and rebuilt today, and its no longer working.

Noticed in the patch (v5) in bug 601022.
Cameron, is this down to bug 477171 landing?

Blair can you create a simple testcase please?
Miiiight also be bug 601999 or bug 587336.  (/me slaps self for pushing those + bug 477171 together & making regression-window-tracking for this bug a little tougher)
I just tried rev 09d926c185f6 (a couple of days ago) plus with bug 601022 applied, so that's with neither my bug 477171 or the others dholbert checked in together applied, and I could still reproduce the problem.
blocking2.0: --- → ?
Version: unspecified → Trunk
Status: NEW → ASSIGNED
I've discovered that loading the extension manager (with bug 601022 applied) from a chrome URL makes the images appear as they should. Its only when loaded as an about: URL that the images will the SVG filter disappears.

So here's a simplified test case. With this extension installed:
* Load chrome://bug611241/content/testcase.xul - both images will appear
* Load about:bug611241 - only the image without the SVG filter will appear (but the other will still take up space)
Isn't that just a duplicate of the general bug about SVG element references not working with URI schemes (like about:) that don't support references?
(In reply to comment #5)
> Isn't that just a duplicate of the general bug about SVG element references not
> working with URI schemes (like about:) that don't support references?

I have little experience with SVG, but doesn't that only apply if the SVG element is accessed via such a URI? In this case, the SVG element is always accessed via a chrome:// URI. It's only the page that has an about: URI.
Oh, I see.  Hmm.  That's odd.  I assume that this about: URI has the system principal, so you don't see any checkLoadURI errors in your error console?
(In reply to comment #7)
> I assume that this about: URI has the system principal, 

Yes.

> so you don't see any checkLoadURI errors in your error console?

I've never seen any such errors, no.
Keywords: regression
No longer blocks: 614983
Duplicate of this bug: 614983
I can reproduce this using Blair's attached extension for as far back as the "about:bug611241" page will load -- that's back to Sept. 11 2010.

Pushlog range for "about:bug611241" no longer loading:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=79d0beec27b5&tochange=73ab2c3c5ad9
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b6pre) Gecko/20100910 Firefox/4.0b6pre
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre

Removing 'regression' keyword, as it looks like this has been broken for as far back as the testcase is loadable.  (But feel free to restore it if I'm misunderstanding something.)

(I'm guessing that Blair's "The same code worked a week or two ago" assertion in Comment 0 was actually due to a chrome:// vs. about: URI difference, per comment 4.)
Keywords: regression
Summary: xul:image with a SVG filter won't display → xul:image with a SVG filter won't display on "about:" pages
Actually, disregard the range I gave in comment 10 -- when I started (again) with a fresh profile and installed Blair's extension, I could go back further.  I'm not sure what caused the 9/10->9/11 boundary in my previous new-profile -- odd.

In any case, now the extension successfully registers its about: page (and reproduces the bug) as far back as 2010-07-02.  The previous day's nightly loads the chrome:// URL fine, but rejects the about: one ("The URL is not valid and cannot be loaded.").

Pushlog range for that is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=82edf5bd1abe&tochange=c173731c9d90
I'm guessing that started working due to bsmedberg's mega-push that day (e.g. "ec1c7f00476d Allow a manifest to register contracts and cids in any order[...]")

In any case, my conclusion from comment 10 still stands (not a regression).
I put a breakpoint in "nsExternalResourceMap::RequestResource" -- we hit it for the chrome:// version, but not for the about: URL.

Are about: pages restricted from loading external resources in some way?
Aha: so for about: version, we bail out inside of "nsReferencedElement::Reset" before we get to the "doc->RequestExternalResource" part, because our own document doesn't have a URL.  Relevant code:
> 78 nsReferencedElement::Reset(nsIContent* aFromContent, nsIURI* aURI,
[...]
> 142   nsCOMPtr<nsIURL> documentURL = do_QueryInterface(doc->GetDocumentURI());
> 143   if (!documentURL)
> 144     return;
> 145 
> 146   if (!EqualExceptRef(url, documentURL)) {
> 147     nsRefPtr<nsIDocument::ExternalResourceLoad> load;
> 148     doc = doc->RequestExternalResource(url, aFromContent, getter_AddRefs(load));

In the about: version, we return early at line 144, because documentURL is null.  It looks like we only care about documentURL for the purpose of checking whether the external-resource URL matches it, though -- so maybe we can just drop that early-return...?
Posted patch fix v1 (obsolete) — Splinter Review
This patch (dropping the early-return) fixes it.

We might still need to do some equivalent of the "EqualsExceptRef" check, though, to keep us from loading an external document for same-document URI references in an about: page...  (e.g. "filter: url(#myLocalRef)")
Posted patch fix v1bSplinter Review
(In reply to comment #14)
> Created attachment 498433 [details] [diff] [review]
> We might still need to do some equivalent of the "EqualsExceptRef" check,
> though, to keep us from loading an external document for same-document URI
> references in an about: page...  (e.g. "filter: url(#myLocalRef)")

Nope, we don't -- we convert local fragments to URLs by that point -- and even if we happened to not do that, we'd bail out early in this method due to the referenced-element URI ('#foo' or 'about:mypage#foo') failing to QI to a URL. (since those aren't valid URLs)
Attachment #498433 - Attachment is obsolete: true
Attachment #498454 - Flags: review?(bzbarsky)
(new patch just adds some clarifying comments)
Comment on attachment 498454 [details] [diff] [review]
fix v1b

>+  // NOTE: aURI shouldn't be just a fragment identifier ('#foo') at this point.
>+  // But if it happens to be, then we'll take the early-return here, because
>+  // it's not a valid URL.

That comment doesn't really make sense.  aURI is an nsIURI.  It can't possibly be "just a fragment identifier" no matter what.  Just take this out, please.

>+  // We've checked that our referenced |url| is a valid nsIURL, so it must be
>+  // more than just a fragment ID like '#foo'. (That wouldn't be a valid URL.)
>+  // So, if |documentURL| is null/invalid (e.g. for "about:" pages), then we
>+  // know that |url| refers to something in a *different* (external) document.

I would phrase this like so:

  // We've already checked that |url| is an nsIURL.  So if the document URI is
  // not an nsIURL then |url| is certainly not going to be pointing to the same
  // document as the document URI.

I would prefer to not bake currently-true-but-we-want-to-change-them details about how about: works into the comments here.

r=me with those comment changes.
Attachment #498454 - Flags: review?(bzbarsky) → review+
Landed: http://hg.mozilla.org/mozilla-central/rev/95bbe1076dcb
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
OS: Windows 7 → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b9
Depends on: 621253
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20101228 Firefox/4.0b9pre

Would an automated test be possible?
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
(In reply to comment #19)
> Would an automated test be possible?

I think so -- we'd just need to set up a testcase (with remote SVG filters) that's registered as an "about:" URI, and visit it at that URI during the test.  (Not having worked on much frontend code, I'm not sure how best to do that in an automated test...)
You need to log in before you can comment on or make changes to this bug.