Closed Bug 1301385 Opened 4 years ago Closed 3 years ago

input type='search' subrole on OS X is wrong

Categories

(Core :: Disability Access APIs, defect, P5)

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: surkov, Assigned: fedepad)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

regression from bug 1137714, see [1]

Accessible::IsSearchField() should check for nsGkAtoms::search value instead nsGkAtoms::textInputType

[1] https://hg.mozilla.org/mozilla-central/rev/5c47eaffd147#l5.17
(In reply to alexander :surkov from comment #0)
> regression from bug 1137714, see [1]
> 
> Accessible::IsSearchField() should check for nsGkAtoms::search value instead
> nsGkAtoms::textInputType
> 
> [1] https://hg.mozilla.org/mozilla-central/rev/5c47eaffd147#l5.17

Hi, as far as I've seen from the codebase, there is no Accessible::IsSearchField() 
but in the file you point to there is Accessible::IsSearchbox().
In this function you suggest to change nsGkAtoms::textInputType to nsGkAtoms::search. 
But shouldn't be nsGkAtoms::searchbox instead of your suggested nsGkAtoms::search? 
Is this the only change to be made?
If that's the only change to be made I will prepare a patch and push for review.

Thanks in advance for your reply.
(In reply to Federico Padua (fedepad) from comment #1)
> (In reply to alexander :surkov from comment #0)
> > regression from bug 1137714, see [1]
> > 
> > Accessible::IsSearchField() should check for nsGkAtoms::search value instead
> > nsGkAtoms::textInputType
> > 
> > [1] https://hg.mozilla.org/mozilla-central/rev/5c47eaffd147#l5.17
> 
> Hi, as far as I've seen from the codebase, there is no
> Accessible::IsSearchField() 
> but in the file you point to there is Accessible::IsSearchbox().

correct

> In this function you suggest to change nsGkAtoms::textInputType to
> nsGkAtoms::search. 
> But shouldn't be nsGkAtoms::searchbox instead of your suggested
> nsGkAtoms::search? 

no, the check is supposed to detect input@type="search", see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Input#Form_%3Cinput%3E_types

> Is this the only change to be made?

yes, we don't have OS X specific tests yet, so no tests for the change

> If that's the only change to be made I will prepare a patch and push for
> review.

thanks!
Just pushed a patch for review.
Comment on attachment 8849558 [details]
Bug 1301385 - input type='search' subrole on OS X is wrong;

https://reviewboard.mozilla.org/r/122338/#review124452

r=me, thanks
Attachment #8849558 - Flags: review?(surkov.alexander) → review+
Comment on attachment 8849558 [details]
Bug 1301385 - input type='search' subrole on OS X is wrong;

https://reviewboard.mozilla.org/r/122338/#review124452

You're welcome! You can trigger try builds or other stuff because I can't do it!
This bug appears to have a reviewed patch, shame it stalled out. Can someone get this committed?
Assignee: nobody → federico_padua
https://hg.mozilla.org/mozilla-central/rev/7b39997a1870
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.