1490026 - Logged out users can trigger updates with keyboard

Status: RESOLVED FIXED

(Webtools Graveyard :: Pontoon, enhancement, P3)

Description

[[DEACTIVATED] Adrian Gaudebert]

It is possible for a non-signed-in user to trigger a translation update (though it fails on the server) on the translate page. Even though that is risk-less, we should still not allow that to happen.

Steps to reproduce (courtesy of Matjaz in bug 1483577#6):

1. Go to any translate page, e.g.:
https://mozilla-pontoon-staging.herokuapp.com/sl/firefox/browser/branding/official/brand.dtd/?string=74127

2. Make sure you're logged out.

3. Press Tab 3 times to focus the translation field.

4. Press Enter to submit translation.

Comment 1

[Skaiste]

Hi, I’m an Outreachy applicant, could I work on this issue?

Comment 2

[Matjaz Horvat [:mathjazz]]

Skaiste, the bug is now assigned to you! Please let me know if you have any questions, either here or in #pontoon on IRC.

You can start by installing Pontoon locally using Docker.

Once you have a working local environment, please make sure to read our contribution docs. It contains information on how to style code, how to run tests, how to name your commits, etc. All the things you need to know if you want your work to be merged into Pontoon!

Comment 3

[Skaiste]

Hi Matjaz,

I got it running locally on my machine. Found out that you don’t need to press tab button three times to trigger the bug after pressing enter. One time pressing tab and then enter also triggers the bug same way.

I tried changing some simple things in the website to check if it’s all working together, but it doesn’t. For example I tried to change first paragraph in InteractiveTour.js file and make it different, but it stayed the same. I have the server running with make run is there is anything else I would need to do?

Really nice project!
Looking forward to continue investigate

Comment 4

[Matjaz Horvat [:mathjazz]]

(In reply to Skaiste from comment #3)

I got it running locally on my machine. Found out that you don’t need to press tab button three times to trigger the bug after pressing enter. One time pressing tab and then enter also triggers the bug same way.

True! We just finished rewriting the translation page and the error is now reproducible slightly differently. Sorry for not updating that in the bug before.

We'll need to disable the "Enter" keyboard shortcut for logged in users and read-only projects, same as we do for the editor menu:
https://github.com/mozilla/pontoon/blob/master/frontend/src/modules/fluenteditor/components/Editor.js#L176

I tried changing some simple things in the website to check if it’s all working together, but it doesn’t. For example I tried to change first paragraph in InteractiveTour.js file and make it different, but it stayed the same. I have the server running with make run is there is anything else I would need to do?

To change localizable strings you'll actually have to change them in the public/static/locale/en-US/translate.ftl and run make build (which will take time). So to quickly test that you are actually making changes, you can trying removing the entire Localized#interactivetour-InteractiveTour--intro-title block instead.

Comment 5

[Skaiste]

(In reply to Matjaz from comment #4)

To change localizable strings you'll actually have to change them in the public/static/locale/en-US/translate.ftl and run make build (which will take time). So to quickly test that you are actually making changes, you can trying removing the entire Localized#interactivetour-InteractiveTour--intro-title block instead.

I'm still having problems getting to see changes in browser. I have make run running, make build-frontend-w and make flow. I did make build and I still didn't see changes.

I tried make test-frontend running and after I changed some test it showed in Terminal that it failed, so this part seems to work.

I tried removing entire Localized#interactivetour-InteractiveTour--intro-title block, but maybe I don't know exactly where to find it. Is this interactive tour is this one I see in this url : http://localhost:8000/en-GB/tutorial/playground/?string=1 ?

I tried commenting out many other places in react code frontend folder, but didn't succeed to get any changes on the browser. Is there any other command I need to be running?

Comment 6

[Matjaz Horvat [:mathjazz]]

Skaiste, the interactive tour is indeed the one on the link you specified. If you removed that element, the title on the first step should disappear.

I've heard complaints from other people about having the same issue, but I haven't been able to figure out what the problem is. I'm adding needinfo for :adrian - the master of our frontend code.

Comment 7

[[DEACTIVATED] Adrian Gaudebert]

Hi Skaiste,

The problem here is due to us having two Translate applications at the moment — we're in the middle of releasing the new one. In the meantime, by default the old translate page is served by default. The code for that app is in Python (django) and JS (jQuery), and can be found around the repo in the pontoon/ folder.

The new app, nicknamed Translate.Next, is not enabled by default. Its code lives in the frontend/ folder, and there are instructions for turning it on there: https://github.com/mozilla/pontoon/blob/master/frontend/README.md

One way to tell if you're using the old or new app is to look at the URL: if it has /translate/ at the beginning, it is the new Translate.Next app, otherwise it's the old translate app.

Comment 8

[Matjaz Horvat [:mathjazz]]

Hi Skaiste, are you still working on this bug?

Comment 9

[Skaiste]

(In reply to Matjaz Horvat [:mathjazz] from comment #8)

Hi Skaiste, are you still working on this bug?

Hi Matjaz,

Yes, I am still working on this issue. I am getting changes in my browser and I see the new frontend code. Although the bug seems to behave differently. When I am clicking tabs the focus goes through navigation bar until it comes to "history" part. Then I can see the focus on users email and after one click I can guess that the next place is approve button as then after clicking enter I see an error.

I found when I set disabled={true} for visible buttons after clicking tab the focus skips buttons. Now I am trying to understand how this.props.isActionDisabled works.

Comment 10

[Matjaz Horvat [:mathjazz]]

Thanks for the update!

I'd rather look into the handleShortcuts function and quit early if we're in isReadOnlyEditor is true (also covers the unauthenticated users):
https://github.com/mozilla/pontoon/blob/master/frontend/src/core/editor/components/connectedEditor.js#L134

Comment 11

[Skaiste]

Hi Matjaz, I see that the bug which was assigned at the beginning with new frontend behaves way different. I could’t say there is a bug. Could you guide what are the steps to reproduce it with new frontend? Thank you

Comment 12

[Matjaz Horvat [:mathjazz]]

For example if you go to http://localhost:8000/translate/sl/tutorial/playground/ and have a translation, you select it in the editor, press Tab twice and hit Enter. You'll see horizontal sidebar appear on top of the page, followed by a notification.

Comment 13

[Matjaz Horvat [:mathjazz]]

Hi Skaiste - since we're running short on the Mentored bugs, please note that another applicant will be working on this bug.

Comment 14

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/pontoon/pull/1450

Comment 15

[Skaiste]

(In reply to Matjaz Horvat [:mathjazz] from comment #13)

Hi Skaiste - since we're running short on the Mentored bugs, please note that another applicant will be working on this bug.

Hi Matjaz, just want to update you that I won’t continue working with this issue. Thanks for support so far!

Comment 16

[Matjaz Horvat [:mathjazz]]

Thanks for the update!

Comment 17

[[DEACTIVATED] Adrian Gaudebert]

adngdb merged PR #1450: "Fix bug 1490026 - Disable keyboard triggers when editor is read only" in ac16c86.