Last Comment Bug 701666 - Removing TxObject define and Double typedef from xslt
: Removing TxObject define and Double typedef from xslt
[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]
Depends on:
  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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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 User image :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.

Comment 1 User image 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 User image :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 User image 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 User image 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 User image 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 User image 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 :-)
Comment 8 User image 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: 
(this changeset is there, is just hidden due to the previous push to try)
Comment 10 User image Ed Morley [:emorley] 2011-11-20 14:19:58 PST

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