accessible/src/html/HTMLTableAccessible.cpp(1208) : warning C4305: 'return' : truncation from 'nsresult' to 'bool'

RESOLVED FIXED in mozilla17

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: m_kato, Assigned: ayg)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
don't return NS_ERROR_xxxx and NS_OK for bool.
(Reporter)

Comment 1

5 years ago
Created attachment 644889 [details] [diff] [review]
fix
(Reporter)

Updated

5 years ago
Attachment #644889 - Flags: review?(trev.saunders)
Comment on attachment 644889 [details] [diff] [review]
fix


>   nsresult rv = GetCellAt(0, 0, *getter_AddRefs(cellElement));
>-  NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
>+  NS_ENSURE_SUCCESS(rv, false);
> 
>   nsCOMPtr<nsIContent> cellContent(do_QueryInterface(cellElement));
>-  NS_ENSURE_TRUE(cellContent, NS_ERROR_FAILURE);
>+  NS_ENSURE_TRUE(cellContent, false);

so, in an ideal world we'd use the RETURN_LAYOUT_ANSWEr() macro, or really just make this stuff infalable, but because of the warnings that's tricky so these seem fine.

>   nsIFrame *cellFrame = cellContent->GetPrimaryFrame();
>   if (!cellFrame) {
>-    return NS_OK;
>+    return true;

it seems you should return false here too, why don't oyu? also using the macro here is probably fine.

Comment 3

5 years ago
Falable is fine. I agree, those should be false since we had false as default value (see http://hg.mozilla.org/mozilla-central/annotate/de43aa36ceef/accessible/src/html/nsHTMLTableAccessible.cpp#l1361).
(Reporter)

Updated

5 years ago
Attachment #644889 - Flags: review?(trev.saunders)

Updated

5 years ago
Duplicate of this bug: 782622
Created attachment 652451 [details] [diff] [review]
Patch

Note that this changes the return value from true to false in the first two cases.  The first of those can't be easily made infallible: it might fail in a lot of ways.  The second should really be infallible in principle, but currently we have no way to make it so in practice.  (Really what we'd want is a method nsIDOMElement::AsNative or something that returns dom::Element, but we don't have one yet.)
Assignee: nobody → ayg
Attachment #644889 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #652451 - Flags: review?(trev.saunders)
Comment on attachment 652451 [details] [diff] [review]
Patch

r=me fwiw I'll be redoing all that in bug 781409 rsn anyway :/
Attachment #652451 - Flags: review?(trev.saunders) → review+
Green try except for Windows: https://tbpl.mozilla.org/?tree=Try&rev=fc3eb12e62f4  Windows isn't included because I'm trying to fix Windows build errors in other patches in the series, but it shouldn't behave any differently on Windows.  (The patch isn't listed at the left, you have to go up to the parent several times to find it.)

https://hg.mozilla.org/integration/mozilla-inbound/rev/e9e13da6a652
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/e9e13da6a652
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17

Updated

5 years ago
Depends on: 784137

Updated

5 years ago
No longer depends on: 784137
You need to log in before you can comment on or make changes to this bug.