Closed
Bug 520487
Opened 15 years ago
Closed 15 years ago
Support URI values in SVG/SMIL
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file, 1 obsolete file)
6.73 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
nsStyleAnimation & nsStyleCoord need to be extended to support URI values (e.g. for the "fill", "filter", and "marker-*" properties). The nsCSSValue class already seems to have a simple solution for this -- adding a "URI*" value to its union member-data. Perhaps nsSytleCoord can do something similar?
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → dholbert
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 1•15 years ago
|
||
oops, meant to set status to 'assigned', not 'resolved/fixed'. :)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•15 years ago
|
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 2•15 years ago
|
||
Attachment #409743 -
Flags: review?(dbaron)
Assignee | ||
Comment 3•15 years ago
|
||
Comment on attachment 409743 [details] [diff] [review] patch 1: Add URI support to nsStyleAnimation Sorry for the delay in getting to this. You need pretty much the same change in ComputeDistance that you have in AddWeighted. The nsStyleAnimation::UncomputeValue changes are wrong; UncomputeValue needs to produce the appropriate representation for storing in the nsCSS* structs. In some sense, all the code that has nsCOMPtr<nsIURI> in the style structs is buggy, and it should really be using the appropriate referrer when it loads the resources and the appropriate principal when necessary (when is it necessary)? I think the right thing to do here is: * make the style structs use nsRefPtr<nsCSSValue::URL> where they currently use nsCOMPtr<nsIURI> (to be done in a separate patch under this one) and just adjust the current users to pull the nsIURI* out of that. (This should also include paint server values.) * make your code use nsCSSValue::URL* where it currently uses nsIURI* * just pass the value straight through rather than having all this complexity in UncomputeValue * file bugs on making sure the implementations of those properties actually use the information in the nsCSSValue::URL* structure. And I'd rather do this now since otherwise this patch would further bake in a bad design. I'd like Boris's opinion on whether he agrees with that plan, though. + const nsIURI *uri = + *static_cast<const nsCOMPtr<nsIURI>*>( + StyleDataAtOffset(styleStruct, ssOffset)); This should just be: nsIURI *uri = *static_cast<nsCOMPtr<nsIURI>* const>( StyleDataAtOffset(styleStruct, ssOffset)); with no const_cast later.
Attachment #409743 -
Flags: review?(dbaron) → review-
Comment 5•15 years ago
|
||
> when is it necessary In practice, in cases when someone might care about the principal of the resulting resource and when the URI of the resource is about:blank, javascript:, or data:. Or any extension scheme that sets the right protocol flags.... > I'd like Boris's opinion on whether he agrees with that plan, though. That seems reasonable to me, yes.
Assignee | ||
Comment 6•15 years ago
|
||
(In reply to comment #4) > + const nsIURI *uri = > + *static_cast<const nsCOMPtr<nsIURI>*>( > + StyleDataAtOffset(styleStruct, ssOffset)); > > This should just be: > nsIURI *uri = *static_cast<nsCOMPtr<nsIURI>* const>( > StyleDataAtOffset(styleStruct, ssOffset)); That doesn't work -- I get this error: > error: static_cast from type ‘const void*’ to type ‘nsCOMPtr<nsIURI>* const’ casts away constness I think the original cast is correct, but its result can just be stored in a normal |nsIURI *uri| -- that variable doesn't need to be const. Fixed that here: http://hg.mozilla.org/users/dholbert_mozilla.com/smil-patches/rev/192c5dc8219d (I'll look into the rest of comment 4 later tonight, probably.)
Assignee | ||
Updated•15 years ago
|
Attachment #409743 -
Attachment is obsolete: true
Assignee | ||
Comment 7•15 years ago
|
||
Comment on attachment 409743 [details] [diff] [review] patch 1: Add URI support to nsStyleAnimation Actually, I think "patch 1" here may be entirely unnecessary -- bug 520239 should handle this just as well.
Assignee | ||
Comment 8•15 years ago
|
||
Shifting component to SVG, since now the only changes here are in patch 2, which just touches SMIL-specific code.
Assignee | ||
Updated•15 years ago
|
Summary: Extend nsStyleAnimation to support URI values → Support URI values in SVG/SMIL
Assignee | ||
Comment 9•15 years ago
|
||
Comment on attachment 409747 [details] [diff] [review] patch: enable URI-valued properties (and tests) on SMIL side Requesting review. Patch is trivial -- just uncomments the properties in nsSMILCSSProperty.cpp::IsPropertyAnimatable, and enables their tests.
Attachment #409747 -
Flags: review?(roc)
Assignee | ||
Updated•15 years ago
|
Attachment #409747 -
Attachment description: patch 2: enable URI-valued properties (and tests) on SMIL side → patch: enable URI-valued properties (and tests) on SMIL side
Attachment #409747 -
Flags: review?(roc) → review+
Assignee | ||
Comment 10•15 years ago
|
||
Landed: http://hg.mozilla.org/mozilla-central/rev/2544d0f0d7ef
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•