Eitan Isaacson [:eeejay] Reporter
	Description • 10 years ago

If you swipe into the app with the screen reader on, we land on an object in the center of the screen. This seems to be the focus indicator.

It should be either made useful, or removed from the accessible tree.

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 1 • 10 years ago

It seem to announce 'Graphic Camera'.

NOTE: It might require a device to test.

	bilal
	Comment 2 • 10 years ago

Im intrested in this code could you tell me more about it?

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 3 • 10 years ago

(In reply to bilal from comment #2)
> Im intrested in this code could you tell me more about it?

Hi Bilal, I wonder if you have a device to test the camera with, otherwise it probably won't be possible with the emulator and Firefox Nightly.

	Hossain Al Ikram [:ikram] (QA Contact)
	Comment 4 • 10 years ago

Hello Yura
I have a device and I want to fix this bug if you guide me. I am having a Keon Device

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 5 • 10 years ago

Hi Hossain, sounds good.
Steps to reproduce are pretty much the same as described in comment 1. Basically you need to start the camera app with the screen reader. After that you should be able to swipe left or right until you will see a small square in the middle of the screen and the screen reader saying 'graphic'. This should not happen. You would need to find what that graphic corresponds to in the DOM and ensure that it is actually hidden (with CSS). You can use WebIDE with your device for that.

Let me know if you have any more questions.

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 6 • 10 years ago

Hello Yura, 
I have noticed the small circle(its not a square) in the middle of the screen ... Are you taking about it? See the attachment image.
And can you pls help me in details how can I debug that app by connected my phone with my computer?
I have Fess the css inside the camera app.
I am new in bug fixing :)

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 8 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #6)
> Hello Yura, 
> I have noticed the small circle(its not a square) in the middle of the
> screen ... Are you taking about it? See the attachment image.
> And can you pls help me in details how can I debug that app by connected my
> phone with my computer?
> I have Fess the css inside the camera app.
> I am new in bug fixing :)

Hi Atique, if you have a device that you are playing around with, have you tried connecting it to WebIDE? Here are instructions on how to connect the device: https://developer.mozilla.org/en-US/docs/Tools/WebIDE

If you manage to do that you should be able to see the HTML markup, CSS styles and such. It also lets you select elements on the devices screen: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Select_an_element (instead of the mouse you just use your finger on the screen). This way you should be able to select the element that the screen reader focuses on. Let me know if you manage to connect and if you could select and figure out the element.

I also suspect that it's actually a focus element that is visible in some cases (when the auto focus happens) so we might have to deal with them.

Let me know if you get any further or if you have any more questions. Thanks

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 9 • 10 years ago

Yeah I have successfully run adb and debug the camera app all day long. But couldn't figure out the element. I have deleted almost everything inside the body. Just kept this lines inside the Body.

<div id="view1" class="viewfinder js-viewfinder visible" grid="off" scale-type="fill">

    <div class="viewfinder-frame js-frame" style="width: 360px; height: 480px;">
        <div class="viewfinder-video-container js-video-container" style="width: 480px; height: 360px; transform: rotate(90deg);">
            <video class="viewfinder-video js-video"></video>
        </div>
    </div>
</div>

Even after then the small sqaure  didn't gone. I have also deleted focus.
can you pls check it ?

	Wilson Page [:wilsonpage]
	Comment 10 • 10 years ago

You need to add `aria-hidden="true"` to the `.focus` element.

I would do this in `apps/camera/js/views/focus.js` with something like:

  this.el.setAttribute('aria-hidden', 'true');

Or add it to relevant elements via the the HTML in the focus view's template.

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 11 • 10 years ago

I would suggest fixing focus-ring.css (to avoid accessibility specific solution for this bug). 

It looks like .focus can only be in 2 states visibility-wise (handled through opacity 0 for default, fixed and none states, and opacity 0.8 for focusing, focused and fail states). We can fix this bug by also adding a corresponding visibility property for those to states: visibility: hidden for all states that have opacity 0 and visibility: visible for all states that have opacity of 0.8. This way we end up with 1 solution for both sighted and screen reader users.

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 12 • 10 years ago

s/to states/two states

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 13 • 10 years ago

Hi Atique,

Sorry, I should've given the example in the previous comment:

so in places were we have:

opacity: 0;

You would need to add visibility like this:

opacity: 0;
visibility: hidden;

and alternatively where it is greater than 0: it should be:

opacity: 0.8;
visibility: visible;

Let me know if this makes sense.

Also, let me know if you are comfortable with Github. If so you could have your changes in a branch and once the pull request is issued I could leave my comments inline. Here are some good suggestions for how to go about it: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Submitting_a_Gaia_patch

Thanks,

Yura

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 14 • 10 years ago

(In reply to Yura Zenevich [:yzen] from comment #13)
> Hi Atique,
> 
> Sorry, I should've given the example in the previous comment:
> 
> so in places were we have:
> 
> opacity: 0;
> 
> You would need to add visibility like this:
> 
> opacity: 0;
> visibility: hidden;
> 
> and alternatively where it is greater than 0: it should be:
> 
> opacity: 0.8;
> visibility: visible;
> 
> Let me know if this makes sense.
> 
> Also, let me know if you are comfortable with Github. If so you could have
> your changes in a branch and once the pull request is issued I could leave
> my comments inline. Here are some good suggestions for how to go about it:
> https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/
> Submitting_a_Gaia_patch
> 
> Thanks,
> 
> Yura

Hello Yura, 
I have done this like this(see the attach picture) but even after then the small square didn't gone. Its not working. I am testing with version 2.1 in Keon.

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 16 • 10 years ago

I think the problem is with viewfinder.css
When I am editing the viewfinder.css like this (see the picture-viewfinder.css edit) 
Then  the small square gone... But with that edit the screen also gone black :p  . 

I not sure where the problem is . You are the expert. You know better :) ..I have just noticed that. :)

	Hossain Al Ikram [:ikram] (QA Contact)
	Comment 17 • 10 years ago

As Atique is working on it , I am assigning him on this bug .

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 19 • 10 years ago

Hi Atique,

I was wondering what version of Gaia are you running at the moment. I can't for some reason replicate the bug anymore, so if you are on the latest master, would you be able to post the steps for reproducing the issue that you posted the screenshot for.

Thanks!

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 20 • 10 years ago

Hello Yura, 
so will i sent pull request in focus-ring.css?  
With code - 

opacity: 0;
visibility: hidden;

opacity: 0.8;
visibility: visible;

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 21 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #20)
> Hello Yura, 
> so will i sent pull request in focus-ring.css?  
> With code - 
> 
> opacity: 0;
> visibility: hidden;
> 
> opacity: 0.8;
> visibility: visible;

Hi Atique, well, before we proceed the pull request, are you actually able to reproduce the bug on device with the latest Gaia master? I for some reason am not able to.. If you do, would you please post the steps that you used to reproduce it? Thanks!

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 22 • 10 years ago

I have tested with my keon 2.1 version. Do you mean the code in github is latest gaia? 
In my case the css code didn't work with keon version 2.1. 
I haven't test with the build in github.

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 23 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #22)
> I have tested with my keon 2.1 version. Do you mean the code in github is
> latest gaia? 
> In my case the css code didn't work with keon version 2.1. 
> I haven't test with the build in github.

Yeah, the latest Gaia code (master branch) is what we should probably test against, because otherwise the bug could be already fixed. Have you tried building/installing latest Gaia on the device? Let me know.

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 24 • 10 years ago

Ok :) I will now download the b2g master codes from github.
But I dont no how to flash that in keon device. Can you pls give me any helping link ?

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 25 • 10 years ago

No - *know

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 26 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #24)
> Ok :) I will now download the b2g master codes from github.
> But I dont no how to flash that in keon device. Can you pls give me any
> helping link ?

For sure:
First you would have to install latest image that you can download from here:
http://downloads.geeksphone.com/ Pick keon nightly from there
After that, assuming you already cloned Gaia repository, you can install truly latest gaia here using:
make install-gaia command (https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Build_System_Primer)

You can also make changes to the camera app CSS and to test them you can just re-install the app with the same command:
APP=camera make install-gaia

This a good primer to starting working on Gaia [accessibility] bugs:
http://yzen.github.io/firefoxos/2014/11/28/resources-for-contributing.html

Let me know if I missed something or if you get stuck somewhere :)

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 27 • 10 years ago

Hi Yura, 
Thanks for the tutorial.

Yeah, I have tested with flashing nighty build . The nighty build was version 3.0 .And there was no square in the middle of the screen. The bug can't be reproduced :( 
But I get a part small square right upward side of the screen(beside the menu). Can't give you the screenshot because my camera isn't working now. 
It worked for the first and last time after flashing version 3.0 (nighty) in keon. I have flashed after that several times but camera is not working now, it just get reset continuously and display loading icon. 

can you please check if you can get a part of small square at right upward corner of the screen? 

Thanks 
Atique

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 29 • 10 years ago

Hey Yuri, 
Thank God the camera is running. I have got the screenshot of the bug. See the attachment image(bug.png). Look at the right side upward corner. You will find a small part of that square. 
When I have put - aria-hidden="true" in html then that small part gone (Like previous time). But couldn't try your css solution as the stylesheet isn't coming in webIDE (see img- stylesheet not coming.jpg)  

So, will it has to be solved ? 

Thanks
Atique Ahmed Ziad

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 31 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #30)
> Created attachment 8566482 [details]
> stylesheet not coming.jpg

Hi Atique, regarding the WebIDE, try updating to latest Firefox Nightly and see if it solves your issue.

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 32 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #29)
> Hey Yuri, 
> Thank God the camera is running. I have got the screenshot of the bug. See
> the attachment image(bug.png). Look at the right side upward corner. You
> will find a small part of that square. 
> When I have put - aria-hidden="true" in html then that small part gone (Like
> previous time). But couldn't try your css solution as the stylesheet isn't
> coming in webIDE (see img- stylesheet not coming.jpg)  
> 
> So, will it has to be solved ? 
> 
> Thanks
> Atique Ahmed Ziad

Yes from what I can tell, the small icon you are seeing is there because of the <video> element. I think the solution here (for that issue) is to make it aria-hidden="true" (the video element). Is that what you tried? Let me know if that fixes the problem and if so , feel free to issue a pull request (see the tutorial). Thanks!

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 33 • 10 years ago

Yeah. I have tried that and its working :) 
So, I will edit viewfinder.js ? 
I will write there :  
document.querySelector(".viewfinder-video-container").setAttribute("aria-hidden", "true");

Its working. I have tested.
Is it okey ?

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 34 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #33)
> Yeah. I have tried that and its working :) 
> So, I will edit viewfinder.js ? 
> I will write there :  
> document.querySelector(".viewfinder-video-container").setAttribute("aria-
> hidden", "true");
> 
> Its working. I have tested.
> Is it okey ?

No need to add it in JS, you can simply set the attribute in the template: https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/views/viewfinder.js#L287

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 37 • 10 years ago

Comment on attachment 8567466 [details]
patch

Looks good from the a11y standpoint, forwarding the r? to Wilson. Thanks, Atique!

	Wilson Page [:wilsonpage]
	Comment 38 • 10 years ago

Comment on attachment 8567466 [details]
patch

Look good. Just a small linter fail because the line is longer than 80 characters. Can land one this is fixed :)

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 39 • 10 years ago

Hey Wilson,
 
Please can you tell how can I wrap that?

	Wilson Page [:wilsonpage]
	Comment 40 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #39)
> Hey Wilson,
>  
> Please can you tell how can I wrap that?

Replied in Github :)

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 41 • 10 years ago

Hey wilson, 
I have changed that as you said. Please see now. Take the latest commit :)

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 42 • 10 years ago

(In reply to Atique Ahmed Ziad from comment #41)
> Hey wilson, 
> I have changed that as you said. Please see now. Take the latest commit :)

Could you squash 2 commits into 1 inside your pull request? you can do it like this:
* when you are in your branch run: git rebase -i upstream/master (assuming upstream points to https://github.com/mozilla-b2g/gaia/)
* At this point, git gives you a way to edit all the commits. I normally 'pick' the first one, then choose 's' for squash for the rest, so the rest of the commits are squashed to the first picked commit.
* Once that is done and git is happy, you can the force push the new state of the branch back to GitHub:
git push -f origin {your branch name}

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 44 • 10 years ago

Hello, 
Sorry for messing up that pull request. I have closed that now. And send another pull request. Please review that. :)

	Atique Ahmed Ziad [:atiqueahmedziad] Assignee
	Comment 45 • 10 years ago

Pls review

	Wilson Page [:wilsonpage]
	Comment 46 • 10 years ago

Atique Ahmed Ziad: For future reference you should only have one pull-request attached to a bug. When you follow up review comments you should use the same Git branch:

1. Make change
2. `git commit add path/to/file.js`
3. `git commit --amend`
4. `git push <your-remote> <branch> -f`
5. Reflag reviewer (r?) for next review

	GitHub Autolander Bot
	Comment 47 • 10 years ago

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/a798c9b590dea73795f3526532477df86c885b4d

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 48 • 9 years ago

Comment on attachment 8569852 [details]
pull request

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): improvement, not a bug
[User impact] if declined: screen readers will be able to step into video element that is confusing since controls are implemented elsewhere.
[Testing completed]: on device
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none

	Ryan VanderMeulen [:RyanVM]
	Comment 49 • 9 years ago

v2.2: https://github.com/mozilla-b2g/gaia/commit/7b06bc2ec3fff0baaf348d46e4fdbda1467e8976