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

RESOLVED FIXED in Firefox 57

Status

()

P5
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: surkov, Assigned: fedepad)

Tracking

({good-first-bug})

unspecified
mozilla57
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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
(Assignee)

Comment 1

2 years ago
(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.
(Reporter)

Comment 2

2 years ago
(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!
Comment hidden (mozreview-request)
(Assignee)

Comment 4

2 years ago
Just pushed a patch for review.
(Reporter)

Comment 5

2 years ago
mozreview-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+
(Assignee)

Comment 6

2 years ago
mozreview-review-reply
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!

Comment 7

a year ago
This bug appears to have a reviewed patch, shame it stalled out. Can someone get this committed?
(Reporter)

Updated

a year ago
Assignee: nobody → federico_padua
Priority: -- → P5
https://hg.mozilla.org/mozilla-central/rev/7b39997a1870
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.