666688 - toolkit/components/typeheadfind - compiler warnings on mac

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Joey Armstrong [:joey]]

% uname -a
Darwin banshee.local 10.7.4 Darwin Kernel Version 10.7.4: Mon Apr 18 21:24:17 PDT 2011; root:xnu-1504.14.12~3/RELEASE_X86_64 x86_64

/mozilla/sandbox/gml/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp:567: warning: enumeral mismatch in conditional expression: 'nsITypeAheadFind::<anonymous enum>' vs 'nsITypeAheadFind::<anonymous enum>'
/mozilla/sandbox/gml/toolkit/components/places/History.cpp:1282: warning: ignoring return value of 'bool mozilla::dom::PContentParent::SendNotifyVisited(const IPC::URI&)', declared with attribute warn_unused_result

Comment 1

[Atul Aggarwal]

Attachment: Patch v1

Patch for first issue. This cannot be fixed in any better way (in my knowledge) as the enums are generated from idl files. Enums needs to be clubbed together to fix this issue.
Second issue does not occur now.

Comment 2

[Josh Matthews [:jdm]]

Can we not do this change and wait for bug 585099 instead?

Comment 3

[Atul Aggarwal]

Definitely, I didn't know that such a bug exists. If we can fix the root cause, there is no point in fixing the implications :).

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 556871 [details] [diff] [review]
Patch v1

You shouldn't introduce tabs, and the typical way to fix this warning is

      *aResult = hasWrapped
                  ? static_cast<PRUint16>(FIND_WRAPPED)
                  : static_cast<PRUint16>(FIND_FOUND);

And please ask a toolkit peer: <https://wiki.mozilla.org/Modules/Toolkit#Toolkit> to review the patch.

Comment 5

[Atul Aggarwal]

Attachment: Patch v2

Changes suggested.

Comment 6

[Justin Dolske [:Dolske]]

Err, isn't the implication from comment 2 that the about-to-land bug 585099 will fix this without code changes?

Comment 7

[Justin Dolske [:Dolske]]

Comment on attachment 558557 [details] [diff] [review]
Patch v2

Clearing review on the assumption(?) 585099 eliminates the need for this.

Comment 8

[Atul Aggarwal]

Marking the bug as Resolved/Fixed as the dependent bug fix has been checked-in.