Removing TxObject define and Double typedef from xslt

RESOLVED FIXED in mozilla11

Status

()

Core
XSLT
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ms2ger, Assigned: Atul Aggarwal)

Tracking

Trunk
mozilla11
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=Ms2ger])

Attachments

(1 attachment, 1 obsolete attachment)

40.40 KB, patch
sicking
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
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
(Assignee)

Comment 1

6 years ago
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.
Assignee: nobody → atulagrwl
Attachment #575408 - Flags: review?(Ms2ger)
(Assignee)

Comment 2

6 years ago
https://tbpl.mozilla.org/?tree=Try&rev=4c3f4eeda2c3
(Reporter)

Comment 3

6 years ago
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.
Attachment #575408 - Flags: review?(jonas)
Attachment #575408 - Flags: review?(Ms2ger)
Attachment #575408 - Flags: review+
(Assignee)

Updated

6 years ago
Summary: Remove TxObject define → Removing TxObject define and Double typedef from xslt
(Assignee)

Comment 4

6 years ago
Created attachment 575442 [details] [diff] [review]
Patch v1.01

Change suggested by Ms2ger. (I did the change using sed and hence missed it :))
Attachment #575408 - Attachment is obsolete: true
Attachment #575408 - Flags: review?(jonas)
Attachment #575442 - Flags: review?(jonas)
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 :)
Attachment #575442 - Flags: review?(jonas) → review+
(Assignee)

Comment 6

6 years ago
Thanks Jonas for the review. I will take care of it from next time.
Keywords: checkin-needed

Comment 7

6 years ago
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
Status: NEW → ASSIGNED
Keywords: checkin-needed

Comment 8

6 years ago
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 9

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bbdb6243033
Flags: in-testsuite-
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/7bbdb6243033
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.