934737 - [AccessFu] Error when encountering an input type="date"

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

[AccessFu] ERROR Error handing accessible event:
 Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]
  _addType@resource://gre/modules/accessibility/OutputGenerator.jsm:224
  _generateBaseOutput@resource://gre/modules/accessibility/OutputGenerator.jsm:330
  defaultFunc@resource://gre/modules/accessibility/OutputGenerator.jsm:500
  entry@resource://gre/modules/accessibility/OutputGenerator.jsm:365
  genForObject@resource://gre/modules/accessibility/OutputGenerator.jsm:121
  addOutput@resource://gre/modules/accessibility/OutputGenerator.jsm:61
  genForContext@resource://gre/modules/accessibility/OutputGenerator.jsm:83
  SpeechPresenter_pivotChanged@resource://gre/modules/accessibility/Presentation.jsm:456
  Presentation_pivotChanged@resource://gre/modules/accessibility/Presentation.jsm:563
  handleAccEvent@resource://gre/modules/accessibility/EventManager.jsm:175
  observe@resource://gre/modules/accessibility/EventManager.jsm:597
  moveToPoint@chrome://global/content/accessibility/content-script.js:101

Comment 1

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 934737 patch v1

Comment 2

[Marco Zehe (:MarcoZ)]

Comment on attachment 827200 [details] [diff] [review]
934737 patch v1

OK, getting this approved for Aurora might prove...interesting...But maybe not since this is a completely new ID instead of a changed one.

Comment 3

[Eitan Isaacson [:eeejay]]

Comment on attachment 827200 [details] [diff] [review]
934737 patch v1

Review of attachment 827200 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good for this specific case. I think the get localized string line should be in a try block, maybe with a warning if an exception occurs. I am worried that other input types will keep cropping up.

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 934737 patch v2

Added try/catch.

Comment 5

[Eitan Isaacson [:eeejay]]

Comment on attachment 827596 [details] [diff] [review]
934737 patch v2

Review of attachment 827596 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/jsat/OutputGenerator.jsm
@@ +223,5 @@
> +    try {
> +      aDesc.push(gStringBundle.GetStringFromName(typeName));
> +    } catch (x) {
> +      Logger.logException(x, 'Failed to get a string from a bundle for ' +
> +        typeName);

How about just a Logger.warning with enough info?

@@ +225,5 @@
> +    } catch (x) {
> +      Logger.logException(x, 'Failed to get a string from a bundle for ' +
> +        typeName);
> +    } finally {
> +      return;

This is redundant, no?

Comment 6

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Eitan Isaacson [:eeejay] from comment #5)
> Comment on attachment 827596 [details] [diff] [review]
> 934737 patch v2
> 
> Review of attachment 827596 [details] [diff] [review]:
> -----------------------------------------------------------------
> @@ +225,5 @@
> > +    } catch (x) {
> > +      Logger.logException(x, 'Failed to get a string from a bundle for ' +
> > +        typeName);
> > +    } finally {
> > +      return;
> 
> This is redundant, no?
I think not having finally gives warnings in debug builds.

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 934737 patch v3

Carrying over a r+ from eeejay.

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2225ed86f53e

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2225ed86f53e