1125523 - Update testSelectionHandler, testTextareaSelections, and testInputSelections to Javascript1.8

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Text Selection, defect)

Description

[Mark Capella [:capella]]

These three tests [1][2][3], need:
   1) To be updated to specify type="application/javascript;version=1.8"
   2) To have all use of |var| keywords upgraded to |let|.

This is a pretty simple code change, and I think it's ok to label it a "good first bug", but you'll need basic knowledge of Javascript, own an Android device, and be able to build and test the Android version of Firefox for Mobile devices. (This is easier if you already have a source repo and can build Firefox for desktop).

A skilled first-time contributor should be able to finish this in under two weeks. Others may take longer, as creating your local build and test environments the first time is a challenge of itself. See [4] for overall details, and the robocop section [5] for testing specifics.

Leave comments / questions here, or look for me in IRC as nick:capella, in #introduction and #mobile channels.


[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/roboextender/testSelectionHandler.html?rev=a914814d2054&force=1&mark=7-7
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/roboextender/testTextareaSelections.html?rev=c837afdde124&force=1&mark=7-7
[3] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/roboextender/testInputSelections.html?rev=08ad468d5ed9&force=1&mark=7-7

[4] https://wiki.mozilla.org/Mobile/Fennec/Android
[5] https://wiki.mozilla.org/Mobile/Fennec/Android#Robocop

Comment 1

[Alexander Dimitrov [:alexxdim94]]

I want to work on that. I'm building Firefox for Android right now and will submit a patch after I'm done. :)

Comment 2

[Alexander Dimitrov [:alexxdim94]]

So all I need to do is update "application/javascript" to "application/javascript;version=1.8" and upgrade all occurrences of var to let. Am I right?

Comment 3

[Mark Capella [:capella]]

That's the easy part. Do you have an android device to test with? We need to run tests (of course) to verify your patch.

This is done fairly easily locally as per [5] in the bug description above. If we have to, we can test by pushing your patch to the TRY-server, but if it fails there, it's often harder to debug.

Comment 4

[Alexander Dimitrov [:alexxdim94]]

Yeah, I have an android device. I've done everything up to setting up Android Studio and building and installing Fennec from it. It all works, but I'm a bit confused. How should I edit code from inside Android Studio? I'll try and install Robocop in a few minutes. :) I'll need you to guide me through testing though.

Comment 5

[Mark Capella [:capella]]

You really don't need AS or an IDE for Firefox/Mobile development. And for this bug, which is javascript only, you can't use it, so I wouldn't bother.

Comment 6

[Alexander Dimitrov [:alexxdim94]]

Oh, okay. So I should just go ahead and install Robocop now?

Comment 7

[Alexander Dimitrov [:alexxdim94]]

Attachment: bug-1125523-fix.diff

I'm not sure if I did everything correctly. Do all of these changes have to be in one .diff file or separate ones? I also need help with running the tests locally. :)

Comment 8

[Alexander Dimitrov [:alexxdim94]]

Attachment: bug-1125523-fix.diff

I rebuilt and reran the tests and all return 0 unexpected results. :)

Comment 9

[Mark Capella [:capella]]

Comment on attachment 8555212 [details] [diff] [review]
bug-1125523-fix.diff

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

Can you modify your commit message to include margaret as the reviewer:
Bug 1125523 - Updated tests to specify JS version and upgraded all uses of var to let, r=margaret

Then do a try-server push, and repost with r? to :margaret, and the try-server push link in the comments.

If you don't have access to the try server yet (L1 access) and plan to continue contributing patches, you might like to follow [1] so you can do this and future pushes yourself.

If you'd rather, you can re-post the patch with your updated commit message, and I'll push it for you, then we can get margarets approval.

Let me know what you want to do.

[1] https://www.mozilla.org/en-US/about/governance/policies/commit/

Comment 10

[Alexander Dimitrov [:alexxdim94]]

Attachment: bug-1125523-fix.diff

https://treeherder.mozilla.org/#/jobs?repo=try&author=[e-mail]

Comment 11

[Mark Capella [:capella]]

Comment on attachment 8557495 [details] [diff] [review]
bug-1125523-fix.diff

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

I'm being allowed to bless this Alex, so let's get it finished :)

Comment 12

[Mark Capella [:capella]]

https://hg.mozilla.org/integration/fx-team/rev/e31a48f67a62

Comment 13

[Carsten Book [:Tomcat]]

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