Bug 40133 - AX: need ListItemRole and PresentationalRole
Summary: AX: need ListItemRole and PresentationalRole
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: chris fleizach
URL:
Keywords:
Depends on:
Blocks: 40132
  Show dependency treegraph
 
Reported: 2010-06-03 11:24 PDT by chris fleizach
Modified: 2010-06-14 23:51 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.82 KB, patch)
2010-06-03 11:46 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (7.62 KB, patch)
2010-06-07 14:22 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (7.66 KB, text/plain)
2010-06-07 14:26 PDT, chris fleizach
no flags Details
patch (7.68 KB, patch)
2010-06-07 14:30 PDT, chris fleizach
darin: review-
Details | Formatted Diff | Diff
patch (6.74 KB, patch)
2010-06-14 09:35 PDT, chris fleizach
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2010-06-03 11:24:55 PDT
We need both these roles to be exposed directly to help solve https://bugs.webkit.org/show_bug.cgi?id=40132
Comment 1 chris fleizach 2010-06-03 11:30:01 PDT
no functional changes. just prep work
Comment 2 chris fleizach 2010-06-03 11:46:03 PDT
Created attachment 57796 [details]
Patch
Comment 3 WebKit Review Bot 2010-06-03 20:31:24 PDT
Attachment 57796 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3102006
Comment 4 chris fleizach 2010-06-07 14:22:57 PDT
Created attachment 58083 [details]
patch

this should hopefully work for chomium too
Comment 5 chris fleizach 2010-06-07 14:26:20 PDT
Created attachment 58084 [details]
patch

now with correct changelogs
Comment 6 chris fleizach 2010-06-07 14:30:17 PDT
Created attachment 58086 [details]
patch

for some reason the last patch didn't take
Comment 7 chris fleizach 2010-06-07 14:38:09 PDT
weird only details is showing up
Comment 8 Darin Adler 2010-06-12 19:12:28 PDT
Comment on attachment 58086 [details]
patch

> +Node* AccessibilityRenderObject::node() const
> +{
> +    if (!m_renderer) 
> +        return 0;
> +    return m_renderer->node();
> +}

What is this function for? It's added in the patch and not used.

The patch otherwise seems great. review- because of the seemingly-unrelated change.
Comment 9 chris fleizach 2010-06-14 09:29:01 PDT
(In reply to comment #8)
> (From update of attachment 58086 [details])
> > +Node* AccessibilityRenderObject::node() const
> > +{
> > +    if (!m_renderer) 
> > +        return 0;
> > +    return m_renderer->node();
> > +}
> 
> What is this function for? It's added in the patch and not used.
> 
> The patch otherwise seems great. review- because of the seemingly-unrelated change.

That was part of the follow up patch. Forgot to take it out of this one
Comment 10 chris fleizach 2010-06-14 09:35:16 PDT
Created attachment 58664 [details]
patch

Patch is same as before, but removes the not necessary node() function from this patch
Comment 11 Darin Adler 2010-06-14 13:54:11 PDT
Comment on attachment 58664 [details]
patch

> +        (WebCore::):

Lines like this should be deleted from the ChangeLog. And we should fix prepare-ChangeLog to not generate them.

> +        (RoleEntry::):

Ditto.
Comment 12 chris fleizach 2010-06-14 14:48:51 PDT
http://trac.webkit.org/changeset/61151
Comment 13 Eric Seidel (no email) 2010-06-14 20:15:41 PDT
Seems to have caused a Gtk API test regression:

ERROR:../../WebKit/gtk/tests/testatkroles.c:98:get_child_and_test_role: assertion failed: (child_role == role)
Comment 14 Eric Seidel (no email) 2010-06-14 20:16:14 PDT
Sheriffbot didn't notice because the buildbot seems to have not been notified of this revision.  Not sure why or how.
Comment 15 chris fleizach 2010-06-14 22:53:36 PDT
(In reply to comment #14)
> Sheriffbot didn't notice because the buildbot seems to have not been notified of this revision.  Not sure why or how.

hopefully i can fix this tonite
Comment 16 chris fleizach 2010-06-14 23:51:06 PDT
looks like
http://trac.webkit.org/changeset/61176 fixed it