Closed
Bug 530372
Opened 15 years ago
Closed 15 years ago
Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in SVG SMIL
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: lusian, Assigned: lusian)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
jwatt
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091120 Minefield/3.7a1pre (.NET CLR 3.5.30729)
Build Identifier:
Please see Bug #512106.
Reproducible: Always
Assignee | ||
Comment 1•15 years ago
|
||
Attachment #413878 -
Flags: review?(roc)
Updated•15 years ago
|
Attachment #413878 -
Flags: review?(roc) → review+
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Updated•15 years ago
|
Assignee: nobody → lusian
Updated•15 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 2•15 years ago
|
||
Sorry, rather badly bit-rotted and no longer comes close to applying.
Keywords: checkin-needed
Comment 3•15 years ago
|
||
Applies fine actually (no fuzz needed). It's got dos-style line-endings, though.
Here's the patch passed through the "dos2unix" tool.
Attachment #417383 -
Flags: review+
Comment 4•15 years ago
|
||
(In reply to comment #3)
> Applies fine actually (no fuzz needed). It's got dos-style line-endings,
> though.
(The dos line endings apparently make "hg import" barf. The "patch" tool handles them better, though it spams the warning "Stripping trailing CRs from patch.")
This is still checkin-needed, but use the unix-style one for more successful patch-application.
Keywords: checkin-needed
Comment 5•15 years ago
|
||
That does indeed apply better, and I someday I'll manage to remember that.
However, if you look at try-eef9251596ce in http://tinderbox.mozilla.org/showbuilds.cgi?tree=MozillaTry&maxdate=1261360140&hours=2 that's just this patch, which burned on Linux, Mac, WinMo and WinCE, only building on WinNT, so I contend it's still not checkin-needed.
Keywords: checkin-needed
Comment 6•15 years ago
|
||
Indeed -- the build error is:
> nsSVGFilters.cpp:4806: error: no matching function for call to ‘NS_MAX(double, float)’
That's an error from by this chunk of the patch:
>- cosConeAngle = PR_MAX(cos(limitingConeAngle * radPerDeg), 0);
>+ cosConeAngle = NS_MAX(cos(limitingConeAngle * radPerDeg), 0.f);
I think "0.f" needs to be "0.0" there.
Comment 7•15 years ago
|
||
Here's the same patch, with the fix from comment 6. It builds fine on my linux machine.
Carrying forward r=jwatt
Attachment #417383 -
Attachment is obsolete: true
Attachment #418585 -
Flags: review+
Comment 8•15 years ago
|
||
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1261364064.1261366611.11219.gz
An amusing option, to have it work on Linux and OS X and WinMo and WinCE, and only fail on WinNT, but I still think a version which will actually compile on every platform would be nicer.
Comment 9•15 years ago
|
||
I guess Windows needs some hand-holding to figure out that it needs to use the |double| version of NS_MAX for two double arguments.
Added an explicit "<double>" in this one -- carrying forward r=jwatt.
Attachment #418585 -
Attachment is obsolete: true
Attachment #418605 -
Flags: review+
Comment 11•15 years ago
|
||
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/15d6851096b9.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
You need to log in
before you can comment on or make changes to this bug.
Description
•