Last Comment Bug 776481 - accessible/src/html/HTMLTableAccessible.cpp(1208) : warning C4305: 'return' : truncation from 'nsresult' to 'bool'
: accessible/src/html/HTMLTableAccessible.cpp(1208) : warning C4305: 'return' :...
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla17
Assigned To: Aryeh Gregor (:ayg) (next working March 28-April 26)
: alexander :surkov
: 782622 (view as bug list)
Depends on:
  Show dependency treegraph
Reported: 2012-07-23 04:09 PDT by Makoto Kato [:m_kato]
Modified: 2012-08-20 19:26 PDT (History)
3 users (show)
ayg: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix (1.41 KB, patch)
2012-07-23 04:10 PDT, Makoto Kato [:m_kato]
no flags Details | Diff | Splinter Review
Patch (1.48 KB, patch)
2012-08-16 07:45 PDT, Aryeh Gregor (:ayg) (next working March 28-April 26)
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review

Description User image Makoto Kato [:m_kato] 2012-07-23 04:09:16 PDT
don't return NS_ERROR_xxxx and NS_OK for bool.
Comment 1 User image Makoto Kato [:m_kato] 2012-07-23 04:10:48 PDT
Created attachment 644889 [details] [diff] [review]
Comment 2 User image Trevor Saunders (:tbsaunde) 2012-08-06 19:39:35 PDT
Comment on attachment 644889 [details] [diff] [review]

>   nsresult rv = GetCellAt(0, 0, *getter_AddRefs(cellElement));
>+  NS_ENSURE_SUCCESS(rv, false);
>   nsCOMPtr<nsIContent> cellContent(do_QueryInterface(cellElement));
>+  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 User image alexander :surkov 2012-08-09 20:09:50 PDT
Falable is fine. I agree, those should be false since we had false as default value (see
Comment 4 User image alexander :surkov 2012-08-14 20:18:55 PDT
*** Bug 782622 has been marked as a duplicate of this bug. ***
Comment 5 User image Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-08-16 07:45:11 PDT
Created attachment 652451 [details] [diff] [review]

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.)
Comment 6 User image Trevor Saunders (:tbsaunde) 2012-08-16 08:10:54 PDT
Comment on attachment 652451 [details] [diff] [review]

r=me fwiw I'll be redoing all that in bug 781409 rsn anyway :/
Comment 7 User image Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-08-19 01:30:23 PDT
Green try except for Windows:  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.)
Comment 8 User image Phil Ringnalda (:philor) 2012-08-19 11:24:34 PDT

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