Bug 117730 – [ATK] Added support for aria-required attribute

• Component: Accessibility
• Status: RESOLVED FIXED

[Krzysztof Czech]

The aria-required attribute is used to indicate that user input is required on an element before a form can be submitted. This attribute can be used with any typical HTML form element.

[Krzysztof Czech]

Created attachment 204909 [details]
Patch

[Mario Sanchez Prada]

The patch looks good to me, although I think it might be a good idea to extend it slightly so we also have the isRequired() method implemented in DumpRenderTree (AccessibityUIElementAtk.cpp)

[Krzysztof Czech]

Created attachment 204912 [details]
Patch

[Krzysztof Czech]

(In reply to comment #2)
> The patch looks good to me, although I think it might be a good idea to extend it slightly so we also have the isRequired() method implemented in DumpRenderTree (AccessibityUIElementAtk.cpp)

Sounds good, done.

[chris fleizach]

Comment on attachment 204912 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204912&action=review

Why does these tests need to be efl/gtk only? they look like they could pass on the Mac without issue as well.
There are also a few accessibility "required" tests that already exist. Do those provide the same coverage? Can they be unskilled by GTK?

> LayoutTests/platform/efl/accessibility/aria-required.html:25
> +          var succeeded = obj.isRequired;

Before outputting these results it would be nice to explain what is being tested like
debug("Verify that isRequired is true when aria-required=true");
So that a failing case is easy to diagnose

> LayoutTests/platform/efl/accessibility/aria-required.html:26
> +          shouldBe("succeeded", "true");

for these, it would be preferable to write
shouldBeTrue("obj.isRequired")

which will give more context if they fail

[Krzysztof Czech]

(In reply to comment #5)
> (From update of attachment 204912 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204912&action=review
> 
> Why does these tests need to be efl/gtk only? they look like they could pass on the Mac without issue as well.
Yes you are right, they're all passing on mac as well. Basically, those two tests are already in mac. I guess moving them to LayoutTests/accessibility it's a good idea. I wasn't really sure about moving them. Thanks.

> There are also a few accessibility "required" tests that already exist. Do those provide the same coverage? Can they be unskilled by GTK?
Right, I think there are 3 accessibility "required" tests. Those two I'm moving are passing on GTK/EFL port, other one it's not.
> 
> > LayoutTests/platform/efl/accessibility/aria-required.html:25
> > +          var succeeded = obj.isRequired;
> 
> Before outputting these results it would be nice to explain what is being tested like
> debug("Verify that isRequired is true when aria-required=true");
> So that a failing case is easy to diagnose
> 
> > LayoutTests/platform/efl/accessibility/aria-required.html:26
> > +          shouldBe("succeeded", "true");
> 
> for these, it would be preferable to write
> shouldBeTrue("obj.isRequired")
> 
> which will give more context if they fail

+ var succeeded = !obj.isRequired;
+ shouldBe("succeeded", "true");
I think, this might be change to shouldBeFalse("obj.isRequired") since we are
expecting to be false.

Thanks for your suggestions.

[Krzysztof Czech]

Created attachment 204987 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 204987 [details]
Patch

Rejecting attachment 204987 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 204987, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
/git.webkit.org/WebKit
   7598594..0c9e165  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 151775 = 759859404bea2807d382deb49e3811336870ab76
r151776 = 0c9e16534f2239a101584831d3f9d49671deff40
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.appspot.com/results/922393

[Krzysztof Czech]

Created attachment 205159 [details]
Rebased patch

[WebKit Commit Bot]

Comment on attachment 205159 [details]
Rebased patch

Clearing flags on attachment: 205159

Committed r151831: <http://trac.webkit.org/changeset/151831>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.