Last Comment Bug 345195 - Replace the anonymous <input type='text'> in <input type='file'> by a simple text
: Replace the anonymous <input type='text'> in <input type='file'> by a simple ...
Status: RESOLVED FIXED
: access, dev-doc-needed, regression
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: All All
: -- normal with 8 votes (vote)
: mozilla22
Assigned To: Mounir Lamouri (:mounir)
:
:
Mentors:
Depends on: 855913 396166 857409 857463 859714 890252
Blocks: focusnav 258875 356071 fox3access 469280 762270 838675 847233
  Show dependency treegraph
 
Reported: 2006-07-19 07:04 PDT by Aaron Leventhal
Modified: 2013-07-06 18:46 PDT (History)
45 users (show)
roc: blocking1.9-
roc: wanted1.9+
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (385 bytes, text/html)
2006-07-19 07:06 PDT, Aaron Leventhal
no flags Details
file to test setting focus to an input type=file field programmatically (1.63 KB, text/html)
2008-02-21 11:33 PST, Becky Gibson
no flags Details
wip (6.59 KB, patch)
2008-07-02 23:17 PDT, alexander :surkov
no flags Details | Diff | Splinter Review
Testcase for (1.29 KB, text/plain)
2011-08-24 06:35 PDT, Tristan Daeschner
no flags Details
Patch (11.59 KB, patch)
2013-02-06 09:26 PST, Mounir Lamouri (:mounir)
bzbarsky: review+
Details | Diff | Splinter Review

Description Aaron Leventhal 2006-07-19 07:04:45 PDT
<input type="file"> controls consist of a textfield and a button. The textfield used to be tabbable, and now it isn't. This makes it difficult for a screen reader user to use the form control (read what file is currently listed there, not to mention realize that it's a file input).

If this behavioral change is intentional, I'm not sure why. It makes us different from the other browsers in a less accessible way. Being able to type in the filename without using the file picker is a good thing.
Comment 1 Aaron Leventhal 2006-07-19 07:06:42 PDT
Created attachment 229814 [details]
Testcase
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-07-19 07:52:02 PDT
This was changed in bug 258875.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2006-07-19 10:32:57 PDT
So it sounds like we want something focusable but not editable, right?

Perhaps we should simply use an overflow:auto inline block or something?
Comment 4 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2006-07-19 10:38:09 PDT
The Browse button is still focusable, no?  So given that we can't allow typing in the textfield, why does it need to be focusable?
Comment 5 alexander :surkov 2006-07-19 10:49:25 PDT
(In reply to comment #3)
> So it sounds like we want something focusable but not editable, right?
> 
> Perhaps we should simply use an overflow:auto inline block or something?
> 

I hope no. Previously I copied path from command line and then pasted it into text field. Now I'm forced to spend five minutes to locate needed file.
Comment 6 aaronr 2006-07-19 11:24:49 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > So it sounds like we want something focusable but not editable, right?
> > 
> > Perhaps we should simply use an overflow:auto inline block or something?
> > 
> 
> I hope no. Previously I copied path from command line and then pasted it into
> text field. Now I'm forced to spend five minutes to locate needed file.
> 


You can paste the FQ path into the file dialog.  I do all the time.  On Windows, at least.
Comment 7 Jonas Sicking (:sicking) No longer reading bugmail consistently 2006-07-19 12:04:08 PDT
I think the reason this bug was opened was because it now is impossible to copy the filename (though I must admit I don't see much use for that, but I agree that it'd be good to allow).

I think we should seriously revise the look of the file-control. The safari one is really nice. Basically it looks like ours, except that it doesn't contain a text-field, rather just the filename as text. If the filename can't fit they use centered ellipsis to shorten it. Additionally they show the appropriate file icon next to the text.

When it comes to focusing I think we should treat the whole thing as one unit. Clicking anywhere in it should open the filepicker (as now) and we can make copying the control copy the filename (don't know how easy that is).

Additionally, we could maybe add an item to the context menu that brings up a dialog box with a simple editable textfield. That way it's still possible to both paste and edit the selected filename. Though I'm not sure how important it is given that only tech-savvy users that prefer to use the keyboard rather than the mouse has a big need to type the filename, and such users will probably not like having to go through a context menu. Oppinions welcome.
Comment 8 aaronr 2006-07-19 13:29:49 PDT
(In reply to comment #7)
> I think the reason this bug was opened was because it now is impossible to copy
> the filename (though I must admit I don't see much use for that, but I agree
> that it'd be good to allow).
> 
> I think we should seriously revise the look of the file-control. The safari one
> is really nice. Basically it looks like ours, except that it doesn't contain a
> text-field, rather just the filename as text. If the filename can't fit they
> use centered ellipsis to shorten it. Additionally they show the appropriate
> file icon next to the text.
> 
> When it comes to focusing I think we should treat the whole thing as one unit.
> Clicking anywhere in it should open the filepicker (as now) and we can make
> copying the control copy the filename (don't know how easy that is).
> 
> Additionally, we could maybe add an item to the context menu that brings up a
> dialog box with a simple editable textfield. That way it's still possible to
> both paste and edit the selected filename. Though I'm not sure how important it
> is given that only tech-savvy users that prefer to use the keyboard rather than
> the mouse has a big need to type the filename, and such users will probably not
> like having to go through a context menu. Oppinions welcome.
> 

I agree that if we are not going to exhibit readonly textfield behavior, then we shouldn't display a readonly textfield.  However, if we want to allow copying from the file input, then I believe that the readonly textfield is a cleaner solution than having to trigger a dialog from a context menu.  So I vote that we should fix up file input to behave correctly.  I would argue that most users are already familiar with the readonly textfield and what you can and cannot do with it.

But a control like Mac's wouldn't be all that bad as long as we can do it in such a way that the file input text can be easily differentiated from surrounding text and from its label if it has one.
Comment 9 Daniel Veditz [:dveditz] 2006-07-20 06:36:46 PDT
(In reply to comment #7)
> I think we should seriously revise the look of the file-control. The safari
> one is really nice. Basically it looks like ours, except that it doesn't
> contain a text-field, rather just the filename as text.

fwiw that was bug 314365 which got WONTFIXED.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2006-07-20 08:36:36 PDT
It's not clear to me _why_ it got wontfixed -- no good reasons were given past "that's just the way it is".
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2007-01-18 18:54:35 PST
I think we should consider moving forward with that more mac-like control...
Comment 12 Steve Lee 2007-09-26 06:09:41 PDT
a further observation is the current behaviour is that only the button is in the tab order but you can focus either with the mouse.
Comment 13 Aaron Leventhal 2007-09-27 10:46:18 PDT
Not only that, but you can keep the text frame focused if you click on it, and then hit Escape in the file chooser.

It's reporting that it is focusable.
Comment 14 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-10-24 01:12:48 PDT
The decision was made to not make any more changes in bug 258875. It's not clear to me what else needs to be done here.
Comment 15 Steve Lee 2007-10-24 01:46:37 PDT
On the usability issue issue the Mouse and keyboard UI's are slightly different. This gives users a confusing conceptual model. I see the main issue is that as you can get a caret in the readonly text box it appears you should be able to interact with it in some way but you have really got into a 'dead' state that you must get out of. So should it be focusable or not?

On accessibility side the ro text control is exposed as state:focusable (which agrees with the above behaviour) which could be taken to indicate that an AT can legitimately give it focus - clearly not the intention and again gets a dead state. It also appears as editable text and has an action of 'activate' that does nothing (should pop up the picker?)

So if the usability is going to stay as is then I think the accessibility needs to change to remove IeditableText and make the activate action pop up the picker
Comment 16 Steve Lee 2007-10-24 01:50:24 PDT
To be absolutely clear, if the intention is that the only way of setting the text is through the picker then the a11y API should have the same symantics.
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-11-07 17:37:33 PST
Aaron, can you make the accessibility change here?
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-01-21 21:04:51 PST
Assigning to Aaron for now. This bug will fall off the blocking list if someone doesn't pick it up soon.
Comment 19 Parkhideh 2008-02-12 00:55:44 PST
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b2) Gecko/2007121120 Firefox/3.0b2
JRE version 6 update 10; (build 1.6.0_10-ea-b11)

1- When on a Yahoo page that displays characters for verification; one can set the focus of the input text field by using a mouse click or the tab; see:

http://info.paperlessprocess.com/Firefox/TextField_Gets_Focus.jpg

2- But when the same window is resized to the point that the text field moves its position; the mouse click will not get focus; although tab will; see

info.paperlessprocess.com/Firefox/TextField_cannot_Focus_with_mouse-click.jpg

3- This problem cannot be produced in the main sign on page of Yahoo because resizing the window does not change the position of the text field; it crops it.

4- IE does not have this problem.

Comment 20 Becky Gibson 2008-02-21 11:33:55 PST
Created attachment 304786 [details]
file to test setting focus to an input type=file field programmatically
Comment 21 Becky Gibson 2008-02-21 11:35:19 PST
You can also not programmaticaly set focus to an input type=file.  Test file inputFileFocus.html attached.  Open inputFileFocus.html and tab to the first button (Something to focus before table). With focus on that button, click the "focus first field" button. That should set focus to the input type=file field labeled with "ID file:" - it does not. Focus goes to the document.  IE and FF 2.0.0.12 do not have this problem.

I also would seriously consider changing the behavior of the input field.  
Comment 22 Becky Gibson 2008-02-21 11:36:40 PST
Sorry, last comment should have been:  I would not recomment changing the behavior of the input field from the established standard people are used to in Firefox 2 and IE.  my two cents
Comment 23 Fernando Pereira Silveira 2008-02-22 21:27:47 PST
If the text field is not editable, how can the user remove the reference to some file if he changes his mind?

The user is obligated to send the file?
Comment 24 Jesse Ruderman 2008-02-22 22:22:56 PST
Pressing delete or backspace should clear the field, IMO.  File a bug for that?
Comment 25 Parkhideh 2008-02-24 18:09:59 PST
(In reply to comment #24)
> Pressing delete or backspace should clear the field, IMO.  File a bug for that?
> 
Without a focus, editing keys: ^C ^V ^X or Del and BkSp do not work.  The only way to be sure that nothing is sent is to have a dummy NIL file and reset the address to that file.  (Open button with blank file name does not work either.)
:)

Comment 26 Jesse Ruderman 2008-02-24 23:48:09 PST
Hmm, the text field can be focused using the mouse but not the keyboard.
Comment 27 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-04-16 22:11:11 PDT
I'd like to address the issue of cancelling file upload, but I can't think of a discoverable way to clear the filename.
Comment 28 Boris Zbarsky [:bz] (still a bit busy) 2008-04-16 22:30:25 PDT
Clearing it on cancel from the filepicker is pretty non-discoverable, eh?

I wonder how Safari handles this (if at all).
Comment 29 Jesse Ruderman 2008-04-16 22:34:18 PDT
Delete/Backspace could clear the field even when the button has focus.

I can't find a way to clear a file upload field in Safari on Tiger.
Comment 30 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-04-16 22:38:52 PDT
Pressing Delete when the button has focus isn't very discoverable since that key would normally do nothing when a button is focused, so users probably won't press it. Backspace is even worse since that would normally go back in the history.

Clearing it on cancel from the filepicker would probably be fine for discoverability, since users wanting to clear the field will probably activate the filepicker (on purpose or by accident) and then hit "cancel" when it doesn't help them. But that breaks the normal semantics of "cancel" and could cause users to accidentally clear fields and possibly submit forms with unexpectedly empty fields.
Comment 31 Aaron Leventhal 2008-07-01 03:16:23 PDT
This affects alternate input projects like Steve Lee's Jambu on screen keyboard. A screen reader developer also recently complained about the FOCUSABLE state on the file input's text field -- basically screen readers are alternative input technologies as well.

I'm not sure if the intent of this bug is to fix more than the a11y API mapping. If so it should be split into several bugs.

Here are the a11y API problems:
When you click in the text field it does get focus for a brief moment, before forwarding focus somewhere else, but that seems to be a hack on our part. It's not really supposed to be focusable. 
As a result of it actually being focusable internally it,
1) ends up with the FOCUSABLE state. 
2) it can get the FOCUSED state sometimes (not sure exactly when)
3) we're firing a11y API focus events on the field

This is of course all wrong -- we should fix this in the widget implementation and not try to wallpaper around it in a11y API code. The actual control should not actually be focusable -- just handle the onclick but not be in the focus order.
Comment 32 neil@parkwaycc.co.uk 2008-07-01 03:45:59 PDT
(In reply to comment #31)
> 2) it can get the FOCUSED state sometimes (not sure exactly when)
After cancelling the filepicker.
Comment 33 Aaron Leventhal 2008-07-01 04:30:29 PDT
Here's how I propose fixing it:
Change nsHTMLInputElement::IsHTMLFocusable(PRBool *aIsFocusable, PRInt32 *aTabIndex)
so that it returns *aTabIndex = -1 and PR_FALSE when mType == NS_FORM_INPUT_TEXT and the node is anonymous, and the binding parent content is <input type="file">:
http://mxr.mozilla.org/seamonkey/source/content/html/content/src/nsHTMLInputElement.cpp#2904

Then change onclick for the same situation so that it forwards the click to the file input's button.
Comment 34 alexander :surkov 2008-07-02 23:17:59 PDT
Created attachment 327924 [details] [diff] [review]
wip
Comment 35 alexander :surkov 2008-07-06 23:03:20 PDT
If I get correctly the actual problem is the upload's text field is focusable but when it's focused by click then filepicker dialog is appeared. That confuses screen readers. 

The text field must be focusable otherwise AT can have difficulties to read its content (Marco, please correct me if I'm wrong) and users can have troubles as well if its content is too long (more than size of text filed). So they should to "click" text field, close filepicker dialog and then they can read. I would suggest to remove the click handler showing the filepicker dialog for the text field. That should fix the problem. How does it sound?
Comment 36 Marco Zehe (:MarcoZ) 2008-07-06 23:11:45 PDT
Yes, that sounds good! The textfield containing the file name is marked read-only anyway, so being able to focus it actually does not entail danger of edits anyway. And being able to focus it to review its contents would be a great help esp for Orca which does not use a virtual buffer.
Comment 37 Steve Lee 2008-07-07 00:41:59 PDT
(In reply to comment #35)
> If I get correctly the actual problem is the upload's text field is focusable
> but when it's focused by click then filepicker dialog is appeared. That
> confuses screen readers. 

It also confuses me ;-) 

> I would
> suggest to remove the click handler showing the filepicker dialog for the text
> field. That should fix the problem. How does it sound?

Sounds good. I think the keyboard, mouse and a11y should have similar behaviours as far as the popup is concerned.
Comment 38 Boris Zbarsky [:bz] (still a bit busy) 2008-07-07 09:13:56 PDT
That sounds like a functionality regression for users to me.  Better to drop the textfield altogether and just have text there without a disabled input.
Comment 39 alexander :surkov 2008-07-08 09:16:18 PDT
(In reply to comment #38)
> That sounds like a functionality regression for users to me.  Better to drop
> the textfield altogether and just have text there without a disabled input.
> 

That should be much similar with Safari's upload control. Marco, how does it sound from the point of view of a11y?
Comment 40 Marco Zehe (:MarcoZ) 2008-07-08 09:57:06 PDT
As long as the file name/path is visible once someone okays the "Browse" dialog, I'm fine with it. Virtual Buffers, as used in the Windows screen readers, can copy & paste plain text as well as text within a textbox, so if this text is within normal text I don't have any problem with it, either.
Comment 41 Steve Lee 2008-07-08 10:14:07 PDT
So visually you have to deal with long paths with ... in the middle, tooltips or some other technique. If they are readonly edit boxes they can be hscrolled (even if styled to look like text as Windows does in file property dialogs; strangely I just tried Nautilus and though it lets you move the caret you cant hscroll and it ends with ...)
Comment 42 alexander :surkov 2008-07-16 02:19:25 PDT
Safari's upload shows the file name only and if file name is big then it shows the start and the end of name only. Tooltip can be good. Is it ok to follow the Safari's control?
Comment 43 Denis Ignatenko 2008-07-17 06:54:51 PDT
document.getElementById("afile").focus() seem to be broken in firefox 3.0.1 I'm not sure for 3.0 version but in 2.0.x it worked. Focus by mouse or keyboard in tab order is ok. By the way it's cool that in 3 version textbox to the left of button getting no focus in tab order.  
Comment 44 Robert Longson 2008-09-02 23:58:42 PDT
*** Bug 453377 has been marked as a duplicate of this bug. ***
Comment 45 Marcel 2008-09-15 16:26:11 PDT
(In reply to comment #43)
> document.getElementById("afile").focus() seem to be broken in firefox 3.0.1 I'm
> not sure for 3.0 version but in 2.0.x it worked. Focus by mouse or keyboard in
> tab order is ok. By the way it's cool that in 3 version textbox to the left of
> button getting no focus in tab order.  

yes, I can approve that.
The behaviour is different on every version and should be like on any other form-field.

Firefox 1.0, 1.5, 2.0:
- focus() works for cursor but view will not scroll to where the field is embedded in

Firefox 3.0 und 3.1a1:
- as of this version, neither scrolling nor cursor focus works


fyi:
in Opera 9.5:
scrolling works, but not cursor focus

in IE 6 & IE7:
focus() quite good! Cursor and view will be set correctly!
Comment 46 Aaron Leventhal 2009-01-14 05:02:41 PST
> *** Bug 453377 has been marked as a duplicate of this bug. ***

Actually it isn't. This bug is about the textfield not taking focus, which is a design choice and probably won't be changed. On the other hand, bug 453377 is about fileInput.focus() not setting focus to the browse button.
Comment 47 Micah Gersten 2009-05-20 23:16:38 PDT
Ubuntu Bug:
https://bugs.launchpad.net/ubuntu/+source/firefox-3.0/+bug/209509
Comment 48 Fernando Pereira Silveira 2009-07-21 00:00:39 PDT
I've been watching to this bug and still see no solution in case the user changes his mind and wish to remove the file to not specify anything.

Should I file a specific bug for that?
Comment 49 Jesse Ruderman 2009-07-21 06:55:11 PDT
That's covered by bug 431098.
Comment 50 Fibonacci 2009-09-03 10:36:45 PDT
Would it be too bad if this behaviour were configurable via about:config?
I know this was changed to prevent certain security exploits on Firefox - so let's say the default behaviour would be as it is now. However, if the users know what they're doing and want to change it to the way it was on Fx 2.0, they should be allowed to change it. Also, this would automatically fix bug 431098.
Comment 51 Jesse Ruderman 2009-09-03 10:39:50 PDT
These exploits are not the kind where "knowing what you're doing" is sufficient protection.
Comment 52 Phillip Susi 2010-04-06 10:36:11 PDT
This sounds absurd.  If a script can jam input into the field without the user's permission and/or awareness, then fix the bug in the script engine; don't deny the user's ability to type in the field.
Comment 53 Boris Zbarsky [:bz] (still a bit busy) 2010-04-06 10:40:59 PDT
Phillip, may I suggest reading bug 258875 (all of it), as well as all linked bugs and testcases, before deciding something is absurd?
Comment 54 Phillip Susi 2010-10-25 12:39:50 PDT
I fail to see why you would suggest that.

Find a way to prevent the feature ( typing in the file name ) from being exploited by a malicious web site; don't kill the feature entirely.
Comment 55 Boris Zbarsky [:bz] (still a bit busy) 2010-10-25 15:02:37 PDT
> I fail to see why you would suggest that.

Because multiple people literally spent years looking for a way to prevent the feature from being exploited by a malicious website.  They failed to find one.  Which is very, had you bothered to read the bug I cited.
Comment 56 Tristan Daeschner 2011-08-24 06:35:41 PDT
Created attachment 555388 [details]
Testcase for

Someone suggested that I attach this test to this bug.

<input type="file"> is the only type of input that cannot be triggered via a label.
Comment 57 Anthony Ricaud (:rik) 2011-09-07 03:39:33 PDT
(In reply to Tristan Daeschner from comment #56)
> Created attachment 555388 [details]
> Testcase for
> 
> Someone suggested that I attach this test to this bug.
> 
> <input type="file"> is the only type of input that cannot be triggered via a
> label.

Looks like this is a different bug. You want to trigger the filepicker or focus the "browse…" button with this.
Comment 58 alexander :surkov 2011-10-26 08:12:10 PDT
Mounir, would you willing to take a look at this one? I think the approach everybody feels ok is to follow safari's way (it works both for sighted and blind users).
Comment 59 Damian 2011-11-01 08:55:11 PDT
I like the idea of going the Safari route :) 

On a related note, will this fix also address the issue of the File input not exposing the aria-label and aria-required attributes that are set on the <input> element? At the moment, these seem to be ignored.
Comment 60 Mounir Lamouri (:mounir) 2011-11-01 09:31:49 PDT
(In reply to alexander surkov from comment #58)
> Mounir, would you willing to take a look at this one? I think the approach
> everybody feels ok is to follow safari's way (it works both for sighted and
> blind users).

By that you mean removing the text field and have only a simple text?
Comment 61 alexander :surkov 2011-11-01 14:25:30 PDT
(In reply to Damian from comment #59)

> On a related note, will this fix also address the issue of the File input
> not exposing the aria-label and aria-required attributes that are set on the
> <input> element? At the moment, these seem to be ignored.

No, could you please file new bug?
Comment 62 alexander :surkov 2011-11-01 14:26:03 PDT
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #60)
> (In reply to alexander surkov from comment #58)
> > Mounir, would you willing to take a look at this one? I think the approach
> > everybody feels ok is to follow safari's way (it works both for sighted and
> > blind users).
> 
> By that you mean removing the text field and have only a simple text?

right
Comment 63 Damian 2011-11-02 04:42:24 PDT
(In reply to alexander surkov from comment #61)
> (In reply to Damian from comment #59)
> 
> > On a related note, will this fix also address the issue of the File input
> > not exposing the aria-label and aria-required attributes that are set on the
> > <input> element? At the moment, these seem to be ignored.
> 
> No, could you please file new bug?

Sure, here it is: https://bugzilla.mozilla.org/show_bug.cgi?id=699017
Comment 64 Alice0775 White 2013-01-24 07:31:42 PST
*** Bug 834206 has been marked as a duplicate of this bug. ***
Comment 65 Mounir Lamouri (:mounir) 2013-02-06 09:26:28 PST
Created attachment 710783 [details] [diff] [review]
Patch

I had to use a <xul:label> because I needed an ellipsis in the middle of the text.

Alexander, how would that be regarding a11y? Is using a <xul:label> a bad thing?
Comment 66 alexander :surkov 2013-02-06 21:00:59 PST
Comment on attachment 710783 [details] [diff] [review]
Patch

Mounir, it's better ask Marco to give a try with screen readers. Btw, it'd be great if you provide a try build.
Comment 67 alexander :surkov 2013-02-06 21:03:03 PST
Comment on attachment 710783 [details] [diff] [review]
Patch

btw, this patch would need a fix on a11y side to make a11y mochitests pass
Comment 68 Mounir Lamouri (:mounir) 2013-02-07 07:31:31 PST
(In reply to alexander :surkov from comment #67)
> Comment on attachment 710783 [details] [diff] [review]
> Patch
> 
> btw, this patch would need a fix on a11y side to make a11y mochitests pass

For the moment, I haven't seen any a11y test failure. Do you know which tests could fail?

(In reply to alexander :surkov from comment #66)
> Comment on attachment 710783 [details] [diff] [review]
> Patch
> 
> Mounir, it's better ask Marco to give a try with screen readers. Btw, it'd
> be great if you provide a try build.

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mlamouri@mozilla.com-b61257dc2541/
Comment 69 Boris Zbarsky [:bz] (still a bit busy) 2013-02-07 15:38:49 PST
Comment on attachment 710783 [details] [diff] [review]
Patch

>   // Mark the element to be native anonymous before setting any attributes.
>   mTextContent->SetNativeAnonymous();

That part really needs to be before the SetAttr!

r=me with that fixed
Comment 70 alexander :surkov 2013-02-07 20:58:27 PST
(In reply to Mounir Lamouri (:mounir) from comment #68)
> (In reply to alexander :surkov from comment #67)
> > Comment on attachment 710783 [details] [diff] [review]
> > Patch
> > 
> > btw, this patch would need a fix on a11y side to make a11y mochitests pass
> 
> For the moment, I haven't seen any a11y test failure. Do you know which
> tests could fail?

Do I understand correct that you replaced anonymous html:input to xul:label?
Comment 71 Mounir Lamouri (:mounir) 2013-02-08 06:00:38 PST
(In reply to alexander :surkov from comment #70)
> (In reply to Mounir Lamouri (:mounir) from comment #68)
> > (In reply to alexander :surkov from comment #67)
> > > Comment on attachment 710783 [details] [diff] [review]
> > > Patch
> > > 
> > > btw, this patch would need a fix on a11y side to make a11y mochitests pass
> > 
> > For the moment, I haven't seen any a11y test failure. Do you know which
> > tests could fail?
> 
> Do I understand correct that you replaced anonymous html:input to xul:label?

Indeed because xul:label supports ellipsis in the middle (with @crop) but CSS do not support that.
Comment 72 Marco Zehe (:MarcoZ) 2013-02-08 06:37:26 PST
Comment on attachment 710783 [details] [diff] [review]
Patch

This patch removes the read-only input that holds the path and name of the chosen file. In a regular nightly build, this is accessible to both JAWS and NVDA. In the try build, using the first testcase above, or any other file input, the path and file name are no longer accessible. All that remains after choosing a file is the "Browse..." button, as if no file had been chosen yet. Denying feedback.
Comment 73 alexander :surkov 2013-02-08 17:27:12 PST
(In reply to Marco Zehe (:MarcoZ) from comment #72)
> Comment on attachment 710783 [details] [diff] [review]
> Patch
> 
> This patch removes the read-only input that holds the path and name of the
> chosen file. In a regular nightly build, this is accessible to both JAWS and
> NVDA. In the try build, using the first testcase above, or any other file
> input, the path and file name are no longer accessible. All that remains
> after choosing a file is the "Browse..." button, as if no file had been
> chosen yet. Denying feedback.

the fix will be rather on a11y side but I agree this shouldn't go until we have accessibility solution for this.

Jamie, I think we need your help. Can you take a look please at try build:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mlamouri@mozilla.com-b61257dc2541/
and say how we should expose the input@type="file" to AT to make it working with NVDA?
Comment 74 Mounir Lamouri (:mounir) 2013-02-09 09:27:07 PST
There are two issues:
- the button is now on the left instead of the right, it is the first child instead of the second;
- the value is shown in a xul:label instead of a html:input which changes a few stuff especially its role is no longer "ENTRY".
Comment 75 Mounir Lamouri (:mounir) 2013-02-10 13:35:19 PST
BTW, do we have to expose the anonymous nodes to the accessible document? It seems that regarding a11y, it would be as good to simply have one element that has some value and can be open (popup state?). I don't know much about a11y so I might be missing some reasons why we expose those anonymous nodes.
Comment 76 alexander :surkov 2013-02-10 16:49:34 PST
(In reply to Mounir Lamouri (:mounir) from comment #75)
> BTW, do we have to expose the anonymous nodes to the accessible document?

it depends, we expose them when we need them

> It
> seems that regarding a11y, it would be as good to simply have one element
> that has some value and can be open (popup state?). I don't know much about
> a11y so I might be missing some reasons why we expose those anonymous nodes.

the idea is to keep a11y closer to the UI, what the user sees on the screen he hears the same by screen reader. It doesn't necessary contradicts to what you said though. I'm certain we need to have that 'Browse' button, mapping of all other controls can vary I guess.
Comment 77 James Teh [:Jamie] 2013-02-10 22:49:17 PST
(In reply to alexander :surkov from comment #73)
> Jamie, I think we need your help. Can you take a look please at try build:
> and say how we should expose the input@type="file" to AT to make it working
> with NVDA?
The problem is that XUL labels expose the IAccessibleText interface, but the text is empty (IAccessibleText::nCharacters returns 0). If the label text is exposed via IAccessibleText, it should work correctly. Alternatively, you can stop exposing IAccessibleText for these labels altogether, but we'd need to discuss this, as NVDA currently won't fall back to accName for labels.
Comment 78 alexander :surkov 2013-02-10 23:05:56 PST
(In reply to James Teh [:Jamie] from comment #77)
> (In reply to alexander :surkov from comment #73)
> > Jamie, I think we need your help. Can you take a look please at try build:
> > and say how we should expose the input@type="file" to AT to make it working
> > with NVDA?
> The problem is that XUL labels expose the IAccessibleText interface, but the
> text is empty (IAccessibleText::nCharacters returns 0). If the label text is
> exposed via IAccessibleText, it should work correctly.

If we would expose IAccessibleText correctly then would it fix a problem?

Mounir, I assume you use @value attribute on that xul:label. Is it possible to switch to text node instead?

> Alternatively, you
> can stop exposing IAccessibleText for these labels altogether, but we'd need
> to discuss this, as NVDA currently won't fall back to accName for labels.

I'd be great if we end up here having an optimal solution for user experience.
Comment 79 James Teh [:Jamie] 2013-02-10 23:10:32 PST
To clarify, there are two solutions:
1. Expose the text correctly via IAccessibleText; or
2. Stop exposing IAccessibleText altogether on that object (i.e. QueryInterface shouldn't return it). To work with current NVDA, the text would have to be exposed as the value rather than the name if the role is label. We can debate this name/value point, but it's not worth discussing if option 1 is suitable.
Comment 80 neil@parkwaycc.co.uk 2013-02-11 03:05:34 PST
(In reply to alexander surkov from comment #78)
> Mounir, I assume you use @value attribute on that xul:label. Is it possible
> to switch to text node instead?
He can't *switch* to text node because he requires the crop behaviour, but I believe the value attribute takes priority over the text node so that if he provides both then it will show cropped text but obviously I don't know whether the accessibility code will see the text node. (Is it possible to query the accessible tree in DOM Inspector to find out?)
Comment 81 Mounir Lamouri (:mounir) 2013-02-11 03:48:35 PST
(In reply to neil@parkwaycc.co.uk from comment #80)
> (In reply to alexander surkov from comment #78)
> > Mounir, I assume you use @value attribute on that xul:label. Is it possible
> > to switch to text node instead?
> He can't *switch* to text node because he requires the crop behaviour, but I
> believe the value attribute takes priority over the text node so that if he
> provides both then it will show cropped text but obviously I don't know
> whether the accessibility code will see the text node. (Is it possible to
> query the accessible tree in DOM Inspector to find out?)

I tried that and that unfortunately doesn't work :(

(In reply to alexander :surkov from comment #78)
> (In reply to James Teh [:Jamie] from comment #77)
> > (In reply to alexander :surkov from comment #73)
> > > Jamie, I think we need your help. Can you take a look please at try build:
> > > and say how we should expose the input@type="file" to AT to make it working
> > > with NVDA?
> > The problem is that XUL labels expose the IAccessibleText interface, but the
> > text is empty (IAccessibleText::nCharacters returns 0). If the label text is
> > exposed via IAccessibleText, it should work correctly.
> 
> If we would expose IAccessibleText correctly then would it fix a problem?
> 
> Mounir, I assume you use @value attribute on that xul:label. Is it possible
> to switch to text node instead?

I'm using xul:label because there is the crop feature.
Comment 82 alexander :surkov 2013-02-11 05:51:27 PST
(In reply to neil@parkwaycc.co.uk from comment #80)
> (In reply to alexander surkov from comment #78)
> > Mounir, I assume you use @value attribute on that xul:label. Is it possible
> > to switch to text node instead?
> He can't *switch* to text node because he requires the crop behaviour, but I
> believe the value attribute takes priority over the text node so that if he
> provides both then it will show cropped text but obviously I don't know
> whether the accessibility code will see the text node.

Do you mean
<xul:label value="file:///c:/path-to-file">file:///c:/path-to-file</xul:label>
?

but then if @value takes precedence and it gets cropped then it'd be great if text node would be the same as representation of @value attribute. That doesn't happen I assume. 

So crop doesn't give desired effect on
<xul:label>file:///c:/path-to-file</xul:label>
or
<xul:description>file:///c:/path-to-file</xul:description>

> (Is it possible to
> query the accessible tree in DOM Inspector to find out?)

sure, just switch to Accessible Tree viewer in left panel and inspect the accessible tree.
Comment 83 Mounir Lamouri (:mounir) 2013-02-11 10:48:04 PST
(In reply to alexander :surkov from comment #82)
> (In reply to neil@parkwaycc.co.uk from comment #80)
> So crop doesn't give desired effect on
> <xul:label>file:///c:/path-to-file</xul:label>
> or
> <xul:description>file:///c:/path-to-file</xul:description>

None of those worked.
Comment 84 alexander :surkov 2013-02-11 20:21:20 PST
How is @value attribute implemented on xul:label? They don't create a native anonymous content for it, correct?
Comment 85 Mounir Lamouri (:mounir) 2013-02-12 02:35:21 PST
(In reply to alexander :surkov from comment #84)
> They don't create a native anonymous content for it, correct?

AFAICT, that's correct.
Comment 86 David Bolter [:davidb] 2013-02-12 08:47:18 PST
(In reply to James Teh [:Jamie] from comment #79)

> 2. Stop exposing IAccessibleText altogether on that object (i.e.
> QueryInterface shouldn't return it). To work with current NVDA, the text
> would have to be exposed as the value rather than the name if the role is
> label. We can debate this name/value point, but it's not worth discussing if
> option 1 is suitable.

Jamie, regardless of this bug, is this worth doing? (If yes let's get a bug filed)
Comment 87 James Teh [:Jamie] 2013-02-12 13:56:32 PST
(In reply to David Bolter [:davidb] from comment #86)
> > 2. Stop exposing IAccessibleText altogether on that object (i.e.
> > QueryInterface shouldn't return it). To work with current NVDA, the text
> > would have to be exposed as the value rather than the name if the role is
> > label.
> Jamie, regardless of this bug, is this worth doing? (If yes let's get a bug
> filed)
The controversial point is exposing the label text as the value instead of the name, as this will break ATs that rely on the label text being the name. My understanding is that a decision hasn't yet been made as to whether to expose the text properly via IAccessibleText. If the answer to that is yes, we don't have to worry about this at all.
Comment 88 David Bolter [:davidb] 2013-02-13 06:28:40 PST
I heard from Alex and we can work out the a11y fix/followup separately. Note uplift is Tuesday...
Comment 89 alexander :surkov 2013-02-13 17:05:28 PST
(In reply to David Bolter [:davidb] from comment #88)
> I heard from Alex and we can work out the a11y fix/followup separately. Note
> uplift is Tuesday...

We can't ship Firefox breaking hardly the accessibility of HTML controls. So the fix must be at least on Beta. Having broken HTML file input on nightly and aurora is a bad thing too since we likely loose testers using nighties as daily browser (I think Marco is one of them). But probably that's acceptable if it doesn't take too long.

If uplifting is Thuesday then we likely get inaccessible nightly and aurora both for uncertain period because we still don't have working a11y solution. I would land this patch after uplifting.

In the meantime I'll try to get a11y patch on top of Mounir's patch.
Comment 90 Mounir Lamouri (:mounir) 2013-02-15 02:45:11 PST
Alexander, is there any way I can help? Do you want me to send you a folded patch with all changes related to this (this patch might not apply cleanly on top af tip)?
Comment 91 alexander :surkov 2013-02-15 03:57:58 PST
(In reply to Mounir Lamouri (:mounir) from comment #90)
> Alexander, is there any way I can help? Do you want me to send you a folded
> patch with all changes related to this (this patch might not apply cleanly
> on top af tip)?

yes, it doesn't apply cleanly. something to apply it would be great
Comment 92 Mounir Lamouri (:mounir) 2013-02-17 15:31:59 PST
(In reply to alexander :surkov from comment #91)
> (In reply to Mounir Lamouri (:mounir) from comment #90)
> > Alexander, is there any way I can help? Do you want me to send you a folded
> > patch with all changes related to this (this patch might not apply cleanly
> > on top af tip)?
> 
> yes, it doesn't apply cleanly. something to apply it would be great

I just sent you a patch that should apply on tip.
Comment 93 alexander :surkov 2013-03-03 17:04:39 PST
Mounir, can you upload a new try build and ask Marco for feedback pls (since bug 396166 is fixed)?
Comment 94 Mounir Lamouri (:mounir) 2013-03-03 17:14:30 PST
Comment on attachment 710783 [details] [diff] [review]
Patch

Marco, could you try this build and tell us if <input type='file'> are okay regarding a11y:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mlamouri@mozilla.com-d7c3eda7996a/

Thanks :)
Comment 95 Marco Zehe (:MarcoZ) 2013-03-18 01:28:14 PDT
Comment on attachment 710783 [details] [diff] [review]
Patch

Cancelling feedback request because the accessibility implications are being taken care of in a separate bug.
Comment 97 Ryan VanderMeulen [:RyanVM] 2013-03-27 14:17:39 PDT
https://hg.mozilla.org/mozilla-central/rev/511ea736284e
Comment 98 j.j. 2013-05-23 12:02:14 PDT
for documentation see also bug 52500
Comment 99 geoffk 2013-07-04 02:29:50 PDT
The 'simple text' is unusable when on a black or dark background. To make file inputs usable (and there are many, many sites for which file inputs are an essential part of their functionality) for sites with dark or black backgrounds now requires some non-obvious CSS and targetting for Firefox 22 and no other browser. Firefox is effectively unusable for sites with dark backgrounds and file inputs. PLEASE revert to the previous treatment of file inputs as soon as possible. I considered simply changing browser and telling everyone I know to do so too, but it occurred to me that I owed Firefox more than that.
Let me restate: file inputs are effectively BROKEN for use on sites with dark backgrounds.
Comment 100 alexander :surkov 2013-07-04 05:49:16 PDT
Would you mind please to file separate bug on it (cc Mounir and me)?
Comment 101 ic3b3rg@gmail.com 2013-07-04 05:56:48 PDT
Geoff K,

This is not an issue.  The required CSS is simple and obvious:

<!DOCTYPE html>
<html>
<head>
<style>

  body {background:black}
  #myfile {color:white}

</style>
</head>
<body>
<input id="myfile" type="file">
</body>
</html>
Comment 102 philippe (part-time) 2013-07-04 05:57:54 PDT
(In reply to geoffk from comment #99)

Bug 874600 is supposed to have fixed that problem (and the test case there works correctly in Fx 22).
Comment 103 geoffk 2013-07-04 08:59:36 PDT
No, ic3b3rg, the fix is far from obvious - and certainly not that obvious. Text inputs have a white background by default, so dark text shows up in them. White text on #myfile would make all other text input invisible. That leaves us needing to provide white text for input[type='file'], but dark text for all other browsers and previous versions of FF  - far more complicated than you seem to think, and who knew that there was going to be a need to cater for this? In a nutshell, the new input[type='file'] is writing outside the box so this particular input type on (recent versions only of...!) this particular browser would need to be a different color to all other inputs. The best fix is not to change font color at all; instead, put a background-color on all inputs, including the file uploads:

input {background-color: #FFF}

This puts a white background behind the text of all inputs, including FF22/23. Do you want to tell the rest of the world they need to do that, on future sites and existing ones, or is it perhaps best to make #FFF the default background for future releases of FF input[type='file'] ?
Comment 104 geoffk 2013-07-04 09:28:25 PDT
Philippe, the test case has a background-color of #FFF applied to it in CSS. If the test case had a black background on the form and the CSS had not been written to compensate, the writing would be invisible. That is exactly as now (not) being seen on any dark-backgrounded sites with a file upload... unless browsing with something other than FF22/23, in which case it continues to be as readable as ever. Inside a form input is light, or dark, and you set CSS to contrast with that background. If 'input' text starts being written outside the input box, which is often dark when the input box is light, and vice versa, the contrast will often be dramatically reduced to the point that you cannot see the text.
Summary - you need to show text against the same background tone for an input[file] as for all other inputs, default being white. You can't rely on having the same background outside of a box as inside.
Comment 105 alexander :surkov 2013-07-06 18:46:37 PDT
Note, Geoff filed bug 890252 for the issue. Let's keep discussion there.

Note You need to log in before you can comment on or make changes to this bug.