WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
57854
[GTK] Implement AccessibilityUIElement::{row|column}IndexRange in DRT
https://bugs.webkit.org/show_bug.cgi?id=57854
Summary
[GTK] Implement AccessibilityUIElement::{row|column}IndexRange in DRT
Mario Sanchez Prada
Reported
2011-04-05 09:17:17 PDT
We need this feature to be able to unskip the following test: accessibility/table-cell-spans.html Prior to this we need to get
bug 57463
and
bug 57829
fixed, so setting a dependency there.
Attachments
Patch proposal + unskipped layout test
(7.35 KB, patch)
2011-04-05 09:24 PDT
,
Mario Sanchez Prada
no flags
Details
Formatted Diff
Diff
Patch proposal + unskipped layout test
(7.03 KB, patch)
2011-04-05 09:51 PDT
,
Mario Sanchez Prada
xan.lopez
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Mario Sanchez Prada
Comment 1
2011-04-05 09:24:08 PDT
Created
attachment 88250
[details]
Patch proposal + unskipped layout test Attaching patch proposal
Alexander Pavlov (apavlov)
Comment 2
2011-04-05 09:25:55 PDT
I don't think the
bug 57829
can influence this issue, as the former is related purely to the Web Inspector internals (unless you need to debug live textual node changes in the Elements panel).
Xan Lopez
Comment 3
2011-04-05 09:32:32 PDT
Comment on
attachment 88250
[details]
Patch proposal + unskipped layout test View in context:
https://bugs.webkit.org/attachment.cgi?id=88250&action=review
> Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:510 > + GOwnPtr<gchar> rangeString(g_strdup_printf("{0, 0}"));
This can just be g_strdup.
> Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:551 > + return JSStringCreateWithUTF8CString(rangeString.get());
These two methods are almost exactly identical (only difference seems to be to use column or row in the printf). I think you can refactor it to make both call a common method that does either one depending on a parameter, for instance?
Mario Sanchez Prada
Comment 4
2011-04-05 09:51:07 PDT
Created
attachment 88259
[details]
Patch proposal + unskipped layout test (In reply to
comment #3
)
> (From update of
attachment 88250
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=88250&action=review
> > > Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:510 > > + GOwnPtr<gchar> rangeString(g_strdup_printf("{0, 0}")); > > This can just be g_strdup.
Sure thing.
> > Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:551 > > + return JSStringCreateWithUTF8CString(rangeString.get()); > > These two methods are almost exactly identical (only difference seems to be to use column or row in the printf). I think you can refactor it to make both call a common method that does either one depending on a parameter, for instance?
Done. I've to say I don't usually like writing helper functions with a boolean parameter to distinguish between two possible situations (row or column range?), but I guess in this case, being a _private_ helper function only used from there, it's more than enough (defining an enumeration for this would look to me 'overkill').
Mario Sanchez Prada
Comment 5
2011-04-05 09:54:01 PDT
(In reply to
comment #2
)
> I don't think the
bug 57829
can influence this issue, as the former is related purely to the Web Inspector internals (unless you need to debug live textual node changes in the Elements panel).
Sorry for the noise, Alexander. I meant
bug 57826
not 57829. My mistake. Fixing it now
Xan Lopez
Comment 6
2011-04-05 09:54:35 PDT
Comment on
attachment 88259
[details]
Patch proposal + unskipped layout test View in context:
https://bugs.webkit.org/attachment.cgi?id=88259&action=review
Looks good minus one comment.
> Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:507 > + return false;
I suppose you want to return a string here?
Mario Sanchez Prada
Comment 7
2011-04-05 10:01:23 PDT
(In reply to
comment #6
)
> (From update of
attachment 88259
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=88259&action=review
> > Looks good minus one comment. > > > Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:507 > > + return false; > > I suppose you want to return a string here?
?? Looks like I uploaded a previous version of the patch, before changing that line (I have the right thing locally, I swear!) Anyway, you're right, that should be an string. Thanks for the review!
Mario Sanchez Prada
Comment 8
2011-04-11 11:57:51 PDT
Committed
r83466
: <
http://trac.webkit.org/changeset/83466
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug