No-Jun Park [:njpark] Reporter
	Description • 10 years ago

when it takes input of words that are longer than dozen characters, it times out.  For example, test_browser_search.py in functional/browser will cause following failure when search_text = 'Mozilla Web QA'

Initially thought it is the same issue as Bug 1064299, but failure seems to have a different cause in this case.  It could be the browser App performance issue, since the keyboard response lags noticeably.

    no-jun-mbp:gaia-ui-tests mozilla$ gaiatest --address=localhost:2828 --testvars=gaiatest/b2g-7.json --restart  --type=b2g --fuzz_factor 5  --get-reference-image gaiatest/tests/graphics/test_browser_search_with_Imagecapture.py
    Results will not be posted to Treeherder. Please set the following environment variables to enable Treeherder reports: TREEHERDER_KEY, TREEHERDER_SECRET
    starting httpd
    running webserver on http://192.168.1.162:51755/
    SUITE-START | Running 1 tests
    TEST-START | test_browser_search_with_Imagecapture.py TestBrowserSearch.test_browser_search
    TEST-UNEXPECTED-ERROR | test_browser_search_with_Imagecapture.py TestBrowserSearch.test_browser_search | TimeoutException: TimeoutException: Timed out after 10.2 seconds
     
     
    Traceback (most recent call last):
      File "/usr/local/Cellar/python/2.7.6/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.8.3-py2.7.egg/marionette/marionette_test.py", line 171, in run
        testMethod()
      File "/Users/mozilla/GitRepo/gaia/tests/python/gaia-ui-tests/gaiatest/tests/graphics/test_browser_search_with_Imagecapture.py", line 26, in test_browser_search
        browser = search.go_to_url(search_text)
      File "/Users/mozilla/GitRepo/gaia/tests/python/gaia-ui-tests/gaiatest/apps/search/app.py", line 26, in go_to_url
        return search_panel.go_to_url(url)
      File "/Users/mozilla/GitRepo/gaia/tests/python/gaia-ui-tests/gaiatest/apps/homescreen/regions/search_panel.py", line 35, in go_to_url
        self.wait_for_condition(lambda m: url in self.apps.displayed_app.name)
      File "/Users/mozilla/GitRepo/gaia/tests/python/gaia-ui-tests/gaiatest/apps/base.py", line 56, in wait_for_condition
        Wait(self.marionette, timeout).until(method, message=message)
      File "/usr/local/Cellar/python/2.7.6/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.8.3-py2.7.egg/marionette/wait.py", line 143, in until
        cause=last_exc) 

Device version info:
Gaia      e2d70bee03b5380ac327a145e5d694fb2443f018
Gecko     https://hg.mozilla.org/mozilla-central/rev/0e581e529fd6
BuildID   20140915201445
Version   35.0a1
Device Name 	 flame
FW-Release 	 4.4.2
FW-Incremental 	 eng.cltbld.20140915.232418
FW-Date 	 Mon Sep 15 23:24:28 EDT 2014
Bootloader 	 L1TC10011800

	No-Jun Park [:njpark] Reporter
	Comment 1 • 10 years ago

It seems that go_to_url cannot handle search words with spaces.  if search text is 'Mozilla Web QA,' then in https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/apps/homescreen/regions/search_panel.py#L35 the value of self.apps.displayed_app.name will contain Mozilla%20Web%20QA as part of the text, while url is 'Mozilla Web QA'  Thus the comparison fails and times out.

When the search text does not contain any spaces, like 'MozillaWebQA', then the function completes successfully.

Not sure what's the rule with sanitizing the output for spaces.  Zac, could you comment?

	Zac C (:zac)
	Comment 2 • 10 years ago

(In reply to No-Jun Park [:njpark] from comment #1)
> Not sure what's the rule with sanitizing the output for spaces.  Zac, could
> you comment?

There is no rule, it only matters that the code works properly and doesn't break. 

This is a use case that we haven't hit yet. So can you please file whatever patch you like and we'll test it and make sure it all works.

	No-Jun Park [:njpark] Reporter
	Comment 3 • 10 years ago

patch to fix the space issue

	No-Jun Park [:njpark] Reporter
	Comment 4 • 10 years ago

(In reply to No-Jun Park [:njpark] from comment #3)
> Created attachment 8493325 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304
> 
> patch to fix the space issue

wait, this has bugs, lemme work on the fix

	No-Jun Park [:njpark] Reporter
	Comment 5 • 10 years ago

Code is updated, same pull request as Comment 3, and change is here:

https://github.com/npark-mozilla/gaia/commit/e3760cc44b552fe0ddfccbb1e2135f35d3e7d247

	No-Jun Park [:njpark] Reporter
	Comment 6 • 10 years ago

Code is updated, same pull request as Comment 3, and change is here:

https://github.com/npark-mozilla/gaia/commit/e3760cc44b552fe0ddfccbb1e2135f35d3e7d247

https://tbpl.mozilla.org/?rev=6966fb0b63eeda80bdfbd7e63adce698e47af3a6&tree=Gaia-Try

	Zac C (:zac)
	Comment 7 • 10 years ago

Comment on attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

redirect to Bebe as Bob doesn't work on the project any longer.

	Florin Strugariu [:Bebe]
	Comment 8 • 10 years ago

Comment on attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

As I understand for every string we type in the Search app and we tap the search button on the keyboard (return) a match is done to check is it's a URL

If it's not then it's considered a search term and a browser window will open with the default search provider search for that term.

Because of this I think we can do a urllib.quote() for every time we do a search (tap return/search)

urllib.quote - Replace special characters in string using the %xx escape. Letters, digits, and the characters '_.-' are never quoted. By default, this function is intended for quoting the path section of the URL. The optional safe parameter specifies additional characters that should not be quoted — its default value is '/'.

So basically if we have any special characters in the search term this will convert it to a URL friendly string that we can wait for.

	No-Jun Park [:njpark] Reporter
	Comment 9 • 10 years ago

(In reply to Florin Strugariu [:Bebe] from comment #8)
> Comment on attachment 8493325 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304
> 
> As I understand for every string we type in the Search app and we tap the
> search button on the keyboard (return) a match is done to check is it's a URL
> 
> If it's not then it's considered a search term and a browser window will
> open with the default search provider search for that term.
> 
> Because of this I think we can do a urllib.quote() for every time we do a
> search (tap return/search)
> 
> urllib.quote - Replace special characters in string using the %xx escape.
> Letters, digits, and the characters '_.-' are never quoted. By default, this
> function is intended for quoting the path section of the URL. The optional
> safe parameter specifies additional characters that should not be quoted —
> its default value is '/'.
> 
> So basically if we have any special characters in the search term this will
> convert it to a URL friendly string that we can wait for.

Aha, I didn't know about the optional safe parameter.  it was converting http:// text to % chars.  I'll make the fix.  Thanks!

	No-Jun Park [:njpark] Reporter
	Comment 10 • 10 years ago

Made the change:
https://github.com/mozilla-b2g/gaia/pull/24304
https://github.com/npark-mozilla/gaia/commit/d956e75984db4fd0c6b0cf19904a7023df96b324

Try result link:
https://tbpl.mozilla.org/?rev=1138bcc09d5576a80cca7f57ec19b80b8878c517&tree=Gaia-Try

	No-Jun Park [:njpark] Reporter
	Comment 11 • 10 years ago

New try:
https://tbpl.mozilla.org/?rev=825030a0419e7df5cf860c6a9950858bce1068eb&tree=Gaia-Try

	Florin Strugariu [:Bebe]
	Comment 12 • 10 years ago

Comment on attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

LGTM

R+ when we squash the commits

	Zac C (:zac)
	Comment 13 • 10 years ago

Comment on attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

r+, njpark can you squash the commits into 1 with the correct commit message before you merge it?

	Zac C (:zac)
	Comment 14 • 10 years ago

This hasn't been merged?

njpark if you merged this somewhere else can you post the link to the commit?

	No-Jun Park [:njpark] Reporter
	Comment 15 • 10 years ago

(In reply to Zac C (:zac) from comment #14)
> This hasn't been merged?
> 
> njpark if you merged this somewhere else can you post the link to the commit?

Sorry, pretty new with this. I think I squashed all commits here with the rebase command (I checked it contains all of my changes, and updated latest master branch into the test branch):

https://github.com/npark-mozilla/gaia/commit/292780c12a1e6d10719a5594af611462dbdb2ef1

If I didn't do it right, let me know and I'll ping you tomorrow morning for help.

	Zac C (:zac)
	Comment 16 • 10 years ago

No-Jun, that's merged in the tip but what we wnat to do is squash it into one single commit. then you may need to force push it to overwrite the old commits.

	No-Jun Park [:njpark] Reporter
	Comment 17 • 10 years ago

new pull request for this bug

	No-Jun Park [:njpark] Reporter
	Comment 18 • 10 years ago

try server test failed in Gij - create new ringtone test, but it looks to me this test has been failing intermittently before.  How can I re-trigger the try server test?

	No-Jun Park [:njpark] Reporter
	Comment 19 • 10 years ago

Hi Zac, when you have time, could you let me know what I need to do with this patch at this point?  thanks!

	Zac C (:zac)
	Comment 20 • 10 years ago

No-jun thanks for ni? I didn't notice you'd updated it.

Now we merge! I will do it when Gaia re-opens.

	Zac C (:zac)
	Comment 21 • 10 years ago

Merge this

	Zac C (:zac)
	Comment 22 • 10 years ago

Merged:
https://github.com/mozilla-b2g/gaia/commit/8b7f9b19234c51011452a12e16969232b92fbcb9