Closed Bug 1074224 Opened 10 years ago Closed 10 years ago

[Accessibility] Get rid of [role="button"] in CSS everywhere.

Categories

(Firefox OS Graveyard :: Gaia, defect)

Product:
Firefox OS Graveyard
Component:
Gaia
Platform:
All
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: yzen, Assigned: yzen)

References

Details

(Keywords: access)

Attachments

(2 files, 1 obsolete file)

Github pull request.
(deleted), text/x-github-pull-request
azasypkin
: review+
iliu
: review+
Details
Github pull request for communications app.
(deleted), text/x-github-pull-request
jmcf
: review+
Details 
Roles must not be used as CSS selectors. If they are, they are a subject of misuse, or in case when the role needs to be applied in markup, the styling comes with it, when it should not.
Attached file Github pull request. (obsolete) (deleted) — 
Marking the following reviewers for these parts:
Alive - System app
Ismael - Building blocks
Doug - Communications
Julien - SMS
Arthur - Settings
Kevin - Calendar
Eric - Bluetooth
Attachment #8498236 - Flags: review?(kgrandon)
Attachment #8498236 - Flags: review?(igonzaleznicolas)
Attachment #8498236 - Flags: review?(felash)
Attachment #8498236 - Flags: review?(echou)
Attachment #8498236 - Flags: review?(drs+bugzilla)
Attachment #8498236 - Flags: review?(arthur.chen)
Attachment #8498236 - Flags: review?(alive)
Comment on attachment 8498236 [details]
Github pull request.

Redirecting to Oleg because I'm away for some days
Attachment #8498236 - Flags: review?(felash) → review?(azasypkin)
Comment on attachment 8498236 [details]
Github pull request.

Calendar part looks fine to me. Thanks.
Attachment #8498236 - Flags: review?(kgrandon) → review+
Comment on attachment 8498236 [details]
Github pull request.

Dialer part LGTM. I'm redirecting the Contacts part to Francisco as I'm not a Contacts peer.
Attachment #8498236 - Flags: review?(francisco)
Attachment #8498236 - Flags: review?(drs+bugzilla)
Attachment #8498236 - Flags: review+
Attachment #8498236 - Flags: review?(alive) → review+
Comment on attachment 8498236 [details]
Github pull request.

Redirect the review to Ian Liu, Bluetooth app owner, since this PR is not related to Bluetooth protocol or logic.
Attachment #8498236 - Flags: review?(echou) → review?(iliu)
Comment on attachment 8498236 [details]
Github pull request.

EJ, could you help review the patch? Thanks.
Attachment #8498236 - Flags: review?(arthur.chen) → review?(ejchen)
Comment on attachment 8498236 [details]
Github pull request.

Thanks Yura,

You forgot to change one more line here : https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/style/settings_large.css#L162

Please remember to update the selector on that file. 

r+
Attachment #8498236 - Flags: review?(ejchen) → review+
Attached file Github pull request. (deleted) — 
Carrying over r+ from alive, drs, ejchen, kgrandon,
Attachment #8498236 - Attachment is obsolete: true
Attachment #8498236 - Flags: review?(iliu)
Attachment #8498236 - Flags: review?(igonzaleznicolas)
Attachment #8498236 - Flags: review?(francisco)
Attachment #8498236 - Flags: review?(azasypkin)
Attachment #8498866 - Flags: review?(iliu)
Attachment #8498866 - Flags: review?(igonzaleznicolas)
Attachment #8498866 - Flags: review?(francisco)
Attachment #8498866 - Flags: review?(azasypkin)
Comment on attachment 8498866 [details]
Github pull request.

Redirecting to Jose, since I won't be able to take a look to this quickly.
Attachment #8498866 - Flags: review?(francisco) → review?(jmcf)
Comment on attachment 8498866 [details]
Github pull request.

Thanks Yura! r=me with one nit on GitHub.
Attachment #8498866 - Flags: review?(azasypkin) → review+
Comment on attachment 8498866 [details]
Github pull request.

After applied the patch, the button displays normally in Bluetooth app. r+ with me
Attachment #8498866 - Flags: review?(iliu) → review+
Comment on attachment 8498866 [details]
Github pull request.

can we have an independent PR for the changes in communications?
Attachment #8498866 - Flags: review?(jmcf)
Comment on attachment 8498866 [details]
Github pull request.

Ismael is no longer working on the project ...
Attachment #8498866 - Flags: review?(igonzaleznicolas)
(In reply to Jose Manuel Cantera from comment #12)
> Comment on attachment 8498866 [details]
> Github pull request.
> 
> can we have an independent PR for the changes in communications?

Yes this is why the PR was split by commits by application, see:
https://github.com/yzen/gaia/commit/e03cd419117c749102f8a422c23c9c50c0f8ed3d
Flags: needinfo?(jmcf)
(In reply to Yura Zenevich [:yzen] from comment #14)
> (In reply to Jose Manuel Cantera from comment #12)
> > Comment on attachment 8498866 [details]
> > Github pull request.
> > 
> > can we have an independent PR for the changes in communications?
> 
> Yes this is why the PR was split by commits by application, see:
> https://github.com/yzen/gaia/commit/e03cd419117c749102f8a422c23c9c50c0f8ed3d

Yes but I think it would be better to have it in an independent PR. We did it for other similar patches such as those related to L10N.
Flags: needinfo?(jmcf)
Comment on attachment 8498866 [details]
Github pull request.

Marking for review for the Building Blocks part.
Attachment #8498866 - Flags: review?(fabien)
Comment on attachment 8498866 [details]
Github pull request.

Kaze is gone. Moving the BB review to Arnau.
Attachment #8498866 - Flags: review?(fabien) → review?(rnowmrch)
Comment on attachment 8499613 [details]
Github pull request for communications app.

r=me for the contacts part.

German,

Please could you have a look at the dialer part?

thanks
Attachment #8499613 - Flags: review?(jmcf)
Attachment #8499613 - Flags: review?(gtorodelvalle)
Attachment #8499613 - Flags: review+
Comment on attachment 8499613 [details]
Github pull request for communications app.

Germán is not a dialer peer, and I've already reviewed this. See comment 4 and comment 8.
Attachment #8499613 - Flags: review?(gtorodelvalle)
(In reply to Doug Sherk (:drs) from comment #20)
> Comment on attachment 8499613 [details]
> Github pull request for communications app.
> 
> Germán is not a dialer peer, and I've already reviewed this. See comment 4
> and comment 8.
then that patch can be landed
Comment on attachment 8498866 [details]
Github pull request.

Sorry guys it took me so long to answer, but my gaia is not working properly and need to clone it again.
Without trying it in a phone the patch looks good.
But please, before landing could you test a case I'm not sure you are covering?
In some apps, for instance settings/languages they are using "fake selects" using a span.button:
https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/elements/languages.html#L12
That's why in buttons BB we have 3 cases when applying rules:
button, [role="button"] and .button
if you are replacing .button with .bb-button, you would need to scan all class="* button *" in all apps before we make sure nothing breaks.
Attachment #8498866 - Flags: review?(rnowmrch)
(In reply to Arnau March  [:arnau] from comment #22)
> Comment on attachment 8498866 [details]
> Github pull request.
> 
> Sorry guys it took me so long to answer, but my gaia is not working properly
> and need to clone it again.
> Without trying it in a phone the patch looks good.
> But please, before landing could you test a case I'm not sure you are
> covering?
> In some apps, for instance settings/languages they are using "fake selects"
> using a span.button:
> https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/elements/
> languages.html#L12
> That's why in buttons BB we have 3 cases when applying rules:
> button, [role="button"] and .button
> if you are replacing .button with .bb-button, you would need to scan all
> class="* button *" in all apps before we make sure nothing breaks.

Hi Arnau, I am only removing cases like [role="button"] so it looks like it's all good.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: