Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 1055355 - Give content editable fields ARIA textbox roles. #23013

Merged
merged 1 commit into from Sep 9, 2014

Conversation

eeejay
Copy link
Contributor

@eeejay eeejay commented Aug 19, 2014

No description provided.

@try-server-hook
Copy link

eeejay Eitan Isaacson (eeejay) started tests. Results

</div>
<div id="messages-input" contentEditable="true" name="message" x-inputmode="-moz-sms" class="js-l10n-placeholder">
<div role="textbox" aria-multiline="true" id="messages-input" contentEditable="true" name="message" x-inputmode="-moz-sms" class="js-l10n-placeholder">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does 'textbox' work fine when there are images inside the input (for MMS for example) ?

Wondering also why contentEditable fields are not visible by the screen reader?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the 'textbox' role seems able to select and render the whole compose, but unfortunately it could not double-tap on the area to focus for editing when attachment inside the composer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: please remove the additional space before role

@try-server-hook
Copy link

eeejay Eitan Isaacson (eeejay) started tests. Results

@try-server-hook
Copy link

eeejay Eitan Isaacson (eeejay) started tests. Results

@try-server-hook
Copy link

eeejay Eitan Isaacson (eeejay) started tests. Results

@@ -24,6 +24,7 @@
this.number = (opts.number || '') + '';
this.email = opts.email || '';
this.editable = opts.editable || 'true';
this.role = this.editable ? 'textbox' : 'button';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please also update the unit test part? Because it would be helpful that preventing any possible regression in recipient node role.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@eeejay eeejay force-pushed the bug-1055355 branch 2 times, most recently from e722f5d to 805640c Compare September 3, 2014 17:28
@try-server-hook
Copy link

eeejay Eitan Isaacson (eeejay) started tests. Results

word-wrap: unset;
background: none;
.recipient[contenteditable=true]:focus,
.recipient[contenteditable=true]:hover,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think we really need the hover pseudo class on device, or it's necessary for a11y?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check tomorrow morning, but I think that is important for overriding
building blocks.

On Thu, Sep 4, 2014 at 8:26 PM, steveck-chung notifications@github.com
wrote:

In apps/sms/style/sms.css:

  • word-wrap: unset;
  • background: none;
    +.recipient[contenteditable=true]:focus,
    +.recipient[contenteditable=true]:hover,

nit: I don't think we really need the hover pseudo class on device, or
it's necessary for a11y?


Reply to this email directly or view it on GitHub
https://github.com/mozilla-b2g/gaia/pull/23013/files#r17155618.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, indeed. It is there to be an override for the BB, I assume the hover style gets applied before/after :active? I don't know. In any case, I was playing it safe.

https://github.com/mozilla-b2g/gaia/blob/master/shared/style/input_areas.css#L461

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I see, thanks!

@try-server-hook
Copy link

eeejay Eitan Isaacson (eeejay) started tests. Results

eeejay added a commit that referenced this pull request Sep 9, 2014
Bug 1055355 - Give content editable fields ARIA textbox roles. r=steveck
@eeejay eeejay merged commit a74801e into mozilla-b2g:master Sep 9, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants