Last Comment Bug 701666 - Removing TxObject define and Double typedef from xslt
: Removing TxObject define and Double typedef from xslt
Status: RESOLVED FIXED
[good first bug][mentor=Ms2ger]
:
Product: Core
Classification: Components
Component: XSLT (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Atul Aggarwal
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-11 04:17 PST by :Ms2ger (⌚ UTC+1/+2)
Modified: 2011-11-20 14:19 PST (History)
2 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (40.32 KB, patch)
2011-11-18 00:59 PST, Atul Aggarwal
Ms2ger: review+
Details | Diff | Splinter Review
Patch v1.01 (40.40 KB, patch)
2011-11-18 06:57 PST, Atul Aggarwal
jonas: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2011-11-11 04:17:12 PST
content/xslt/src/base/txCore.h has '#define TxObject txObject'. It would be better if code used the latter form instead.

See

http://mxr.mozilla.org/mozilla-central/ident?i=TxObject&tree=mozilla-central&filter=&strict=1
Comment 1 Atul Aggarwal 2011-11-18 00:59:53 PST
Created attachment 575408 [details] [diff] [review]
Patch v1

Removing the TxObject define as well removing the Double typedef. Patch is somewhat long but it requires only a glance.
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2011-11-18 05:49:53 PST
Comment on attachment 575408 [details] [diff] [review]
Patch v1

>--- a/content/xslt/src/xslt/txNodeSorter.h
>+++ b/content/xslt/src/xslt/txNodeSorter.h
>-    static bool calcSortValue(TxObject*& aSortValue, SortKey* aKey,
>+    static bool calcSortValue(txObject*& aSortValue, SortKey* aKey,
>                                 SortData* aSortData, PRUint32 aNodeIndex);

Reindent the next line while you're here.

Looks good to me, but I'd like Jonas' sign-off.
Comment 4 Atul Aggarwal 2011-11-18 06:57:46 PST
Created attachment 575442 [details] [diff] [review]
Patch v1.01

Change suggested by Ms2ger. (I did the change using sed and hence missed it :))
Comment 5 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-11-18 17:15:15 PST
Comment on attachment 575442 [details] [diff] [review]
Patch v1.01

For patches like this, it's nice if you can attach the script (or list of commands) used to create the patch, in addition to the patch itself.

It's easier to review the script for correctness...

r=me though :)
Comment 6 Atul Aggarwal 2011-11-19 07:47:18 PST
Thanks Jonas for the review. I will take care of it from next time.
Comment 7 Ed Morley [:emorley] 2011-11-19 18:09:56 PST
In my queue with a few other bits that are being sent to try first and then onto inbound :-)

https://tbpl.mozilla.org/?tree=Try&rev=eb96679e6888
Comment 8 Ed Morley [:emorley] 2011-11-19 18:25:26 PST
Bug 687511 included in that try run failed check-sync-dirs, so had to push again:

https://tbpl.mozilla.org/?tree=Try&rev=329bc2d383a3 
(this changeset is there, is just hidden due to the previous push to try)
Comment 10 Ed Morley [:emorley] 2011-11-20 14:19:58 PST
https://hg.mozilla.org/mozilla-central/rev/7bbdb6243033

Note You need to log in before you can comment on or make changes to this bug.