942730 - The send_keys() method of input element (type is number) will raise MarionetteException

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Askeing Yen[:askeing][:fyen]]

### ENV:
Buri
Base Img: V1.2_US_20131115.cfg
Gaia:     c736d91f6a1aada7b81a3d7b7df2635f2f9a655a
Gecko:    http://hg.mozilla.org/mozilla-central/rev/74ab61b8d0f0
BuildID   20131124040202
Version   28.0a1
ro.build.version.incremental=eng.archermind.20131114.105818

### Description:
The send_keys() method of input element (type is number) will raise MarionetteException.
ex: <input type="number" id="data-limit-input" />

TEST-START test_cost_control_ftu.py
test_cost_control_ftu (test_cost_control_ftu.TestCostControlFTU) ... ERROR

======================================================================
ERROR: None
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette_test.py", line 143, in run
    testMethod()
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/gaiatest/tests/functional/cost_control/test_cost_control_ftu.py", line 26, in test_cost_control_ftu
    ftu_step3.select_when_use_is_above_unit_and_value(u'MB', '0.1')
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/gaiatest/apps/cost_control/regions/ftu_step3.py", line 49, in select_when_use_is_above_unit_and_value
    size.send_keys(value)
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 107, in send_keys
    return self.marionette._send_message('sendKeysToElement', 'ok', id=self.id, value=typing)
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 577, in _send_message
    self._handle_error(response)
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 638, in _handle_error
    raise MarionetteException(message=message, status=status, stacktrace=stacktrace)
TEST-UNEXPECTED-FAIL | test_cost_control_ftu.py test_cost_control_ftu.TestCostControlFTU.test_cost_control_ftu | MarionetteException
----------------------------------------------------------------------
Ran 1 test in 55.146s


TEST-START test_cost_control_data_alert_mobile.py
test_cost_control_data_alert_mobile (test_cost_control_data_alert_mobile.TestCostControlDataAlertMobile) ... ERROR

======================================================================
ERROR: None
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette_test.py", line 143, in run
    testMethod()
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/gaiatest/tests/functional/cost_control/test_cost_control_data_alert_mobile.py", line 34, in test_cost_control_data_alert_mobile
    settings.select_when_use_is_above_unit_and_value(u'MB', '0.1')
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/gaiatest/apps/cost_control/regions/settings.py", line 58, in select_when_use_is_above_unit_and_value
    size.send_keys(value)
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 107, in send_keys
    return self.marionette._send_message('sendKeysToElement', 'ok', id=self.id, value=typing)
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 577, in _send_message
    self._handle_error(response)
  File "/var/lib/jenkins/jobs/B2G.master.pvt.hamachi.gaiatest/workspace/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 638, in _handle_error
    raise MarionetteException(message=message, status=status, stacktrace=stacktrace)
TEST-UNEXPECTED-FAIL | test_cost_control_data_alert_mobile.py test_cost_control_data_alert_mobile.TestCostControlDataAlertMobile.test_cost_control_data_alert_mobile | MarionetteException
----------------------------------------------------------------------
Ran 1 test in 62.759s

Comment 1

[Askeing Yen[:askeing][:fyen]]

MV Jenkins failed: http://qa-selenium.mv.mozilla.com:8080/job/b2g.hamachi.mozilla-central.ui/316/
TWCI failed: http://release1-qa.corp.tpe1.mozilla.com:8080/job/B2G.master.pvt.hamachi.gaiatest/64/

Comment 2

[Zac C (:zac)]

~Pushing this back to Marionette team.

This is likely because of the changes jwatt made to input type=number last week.

Jwatt can you point us to the Gecko commit so that A-team can fix Marionette?

Comment 3

[Jonathan Watt [:jwatt]]

You need to read this (more helpful than the commit):

https://groups.google.com/d/msg/mozilla.dev.b2g/HN_9ziODqAY/Hy3BmJ8gMKoJ

Comment 4

[David Burns :automatedtester]

Speaking to JWatt we need to look at the changes in dependencies to Bug 344616 since he doesn't know what changes have been done to EventUtils. :(

Comment 5

[David Burns :automatedtester]

I tracked the issue down to http://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-sendkeys.js#24 throwing an error. It appears that selectionStart can not be set on <input type=number> in Firefox but works in Chrome so think it either hasnt been implemented or there is a bug.

Below is a quick test to show the issue. It should say that it has worked. You can try it at http://jsbin.com/oQAKUZA/2

<!DOCTYPE html>
<html>
<head>
<meta charset=utf-8 />
<title>JS Bin</title>

</style>
</head>
<body>
  <input id='number' type=number>
  <script>
    var message = 'it worked';
    try{ 
      document.getElementById('number').selectionStart = 0;
    }catch (e) {
      message = 'it failed';
    }
      document.getElementsByTagName('body')[0].appendChild(document.createTextNode(message));
  </script>
</body>
</html>

Comment 6

[Jonathan Watt [:jwatt]]

http://www.whatwg.org/specs/web-apps/current-work/multipage/states-of-the-type-attribute.html#number-state-%28type=number%29

says:

  The following IDL attributes and methods do not apply to
  the element: checked, files, selectionStart, selectionEnd,
  selectionDirection,...

Then at:

http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#textFieldSelection

it says:

  For input elements, calling these methods while they don't
  apply, and getting or setting these attributes while they don't
  apply, must throw an InvalidStateError exception.

So it seems like we're doing what the spec requires, and Chrome has a bug.

Comment 7

[Jonathan Watt [:jwatt]]

You should check input.mozIsTextField(false) on the input before messing with selection. That method is declared here:

https://mxr.mozilla.org/mozilla-central/source/dom/webidl/HTMLInputElement.webidl#169

Comment 8

[David Burns :automatedtester]

Raised http://code.google.com/p/chromium/issues/detail?id=324360 after comment 6

Comment 9

[David Burns :automatedtester]

I have marionette populating the input field. I need to add more tests around typing as I am worried that this might break other things. I don't feel we have the converage imho

Comment 10

[David Burns :automatedtester]

Attachment: try: -b do -p linux,macosx64,win32,linux_gecko,linux64_gecko -u marionette,marionette-webapi,gaia-ui-test -t none

Comment 11

[David Burns :automatedtester]

try pushes 

https://tbpl.mozilla.org/?tree=Try&rev=104c8b31bd43
https://tbpl.mozilla.org/?tree=Try&rev=b89898925711

Comment 12

[David Burns :automatedtester]

Comment on attachment 8344067 [details] [diff] [review]
try: -b do -p linux,macosx64,win32,linux_gecko,linux64_gecko -u marionette,marionette-webapi,gaia-ui-test -t none

Noticed a few things I thought I had removed, which won't really affect the way things run. Cleaning them up and will attach new patch in a few minutes

Comment 13

[David Burns :automatedtester]

Attachment: allow Marionette to type into input type=number fields;

Comment 14

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8344074 [details] [diff] [review]
allow Marionette to type into input type=number fields;

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

I think the full test_html_inputs.html file is missing here.

Comment 15

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8344074 [details] [diff] [review]
allow Marionette to type into input type=number fields;

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

::: browser/app/profile/firefox.js
@@ +1332,5 @@
>  // CustomizableUI debug logging.
>  pref("browser.uiCustomization.debug", false);
>  
> +// input type=number MUST BE REMOVED BEFORE LANDING
> +pref("dom.forms.number", true);

What does this do? Was this supposed to be in the patch?

::: testing/marionette/client/marionette/tests/unit/test_text.py
@@ +151,5 @@
> +
> +        result = self.marionette.find_element("id", "result")
> +        # Because the key down gets the result before the input element is
> +        # filled, we're a letter short here
> +        self.assertEqual(result.text, "I like cheese")

Shouldn't this be 'I like chees'? Does this pass locally?

::: testing/marionette/marionette-listener.js
@@ +1687,5 @@
> +            keyCode = "VK_F11";
> +            break;
> +          case '\uE03C':
> +            keyCode = "VK_F12";
> +            break;

Can this be moved to the marionette-sendkeys.js file? I eventually want to share this bit between marionette-server and listener, when we get around to implementing send_keys in chrome, and it seems more appropriate there.

@@ +1704,5 @@
>        sendError("Element is not visible", 11, null, command_id)
>      }
>    }
>    catch (e) {
> +    logger.info("ZOMG ERROR: " + e)

heh, leftover debug log line

Comment 16

[David Burns :automatedtester]

Attachment: allow Marionette to type into input type=number fields;

Comment 17

[David Burns :automatedtester]

(In reply to Malini Das [:mdas] from comment #15)
> Comment on attachment 8344074 [details] [diff] [review]
> allow Marionette to type into input type=number fields;
> 
> Review of attachment 8344074 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/app/profile/firefox.js
> @@ +1332,5 @@
> >  // CustomizableUI debug logging.
> >  pref("browser.uiCustomization.debug", false);
> >  
> > +// input type=number MUST BE REMOVED BEFORE LANDING
> > +pref("dom.forms.number", true);
> 
> What does this do? Was this supposed to be in the patch?

To Enable the sub-DOM stuff you need that pref to see the correct layout. If this isnt there it should just create a normal <input type=text> instead and *just* work when :jwatt switches it on full time.

I have removed it since I dont want to turn that on

> 
> ::: testing/marionette/client/marionette/tests/unit/test_text.py
> @@ +151,5 @@
> > +
> > +        result = self.marionette.find_element("id", "result")
> > +        # Because the key down gets the result before the input element is
> > +        # filled, we're a letter short here
> > +        self.assertEqual(result.text, "I like cheese")
> 
> Shouldn't this be 'I like chees'? Does this pass locally?

Copy paste fail... sorted

> 
> ::: testing/marionette/marionette-listener.js
> @@ +1687,5 @@
> > +            keyCode = "VK_F11";
> > +            break;
> > +          case '\uE03C':
> > +            keyCode = "VK_F12";
> > +            break;
> 
> Can this be moved to the marionette-sendkeys.js file? I eventually want to
> share this bit between marionette-server and listener, when we get around to
> implementing send_keys in chrome, and it seems more appropriate there.
> 

Since marionette-sendkeys.js is licenced to WebDriver currently and I am refactoring a large amount (read removing the webdriver code in favour of our implementation) I would rather move things back to marionette-sendkeys in bug 947490 ( a dependent bug ) and repurpose the licence when fully completed.

> @@ +1704,5 @@
> >        sendError("Element is not visible", 11, null, command_id)
> >      }
> >    }
> >    catch (e) {
> > +    logger.info("ZOMG ERROR: " + e)
> 
> heh, leftover debug log line

Sorted

Comment 18

[David Burns :automatedtester]

Attachment: allow Marionette to type into input type=number fields;

Comment 19

[Malini Das [:mdas] - Away, not checking bugmail]

Thanks, I've added a comment about moving this code to sendkeys in the dependent bug

Comment 20

[David Burns :automatedtester]

landed in https://hg.mozilla.org/integration/mozilla-inbound/rev/b14f5674b64b

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b14f5674b64b

Comment 22

[David Burns :automatedtester]

(In reply to David Burns :automatedtester from comment #8)
> Raised http://code.google.com/p/chromium/issues/detail?id=324360 after
> comment 6

This is now fix http://code.google.com/p/chromium/issues/detail?id=324360#c7

Comment 23

[Ryan VanderMeulen [:RyanVM]]

In my queue for when b2g26 reopens.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dc827be2cab4
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/134c64fb6834