[Webkit-unassigned] [Bug 36128] [Gtk] nameFromChildren is obsolete

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 11 08:46:26 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=36128


Mario Sanchez Prada <msanchez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #58479|                            |review?
               Flag|                            |




--- Comment #9 from Mario Sanchez Prada <msanchez at igalia.com>  2010-06-11 08:46:25 PST ---
Created an attachment (id=58479)
 --> (https://bugs.webkit.org/attachment.cgi?id=58479)
new layout test which should work fine

(In reply to comment #5)
> [...]
> bufferLength is based on the length of the TextIterator. This length seems to incremented in emitText(). emitText() is not getting called in the layout test because TextIterator::handleTextNode() is returning at this point:
> 
>     if (!renderer->firstTextBox() && str.length() > 0) {
>         m_lastTextNodeEndedWithCollapsedSpace = true; // entire block is collapsed space
>         return true;
>     }
> 
> So.... Given the same, simple HTML content, why would a RenderText object fail to have a firstTextBox when run as a layout test, but behave as expected otherwise?

After digging on this issue for some time (as well as trying to figure out how DRT and the whole a11y stack works in WebKit :-)), I think I finally found the reason while this test was failing with DRT but no with Accerciser: it's just that the layout() process has not been completed when you ask for the label's title in the test, so the text box for that piece of text is still not created, resulting in what we already know that is an empty string being returned :-/.

In other words, the failing flow would be:

  1. In the test you just ask for 'child.title' while the layout() still didn't happen
  2. webkit_accessible_text_get_name for the entry boils down to webkit_accessible_text_get_text for the corresponding label, which actually falls back to AccessibilityRenderObject::textUnderElement().
  3. AccessibilityRenderObject::textUnderElement() needs to call to plainText(), which internally ends up creating a new TextIterator for the range passed to plainText().
  4. However, the text iterator created when running DRT is an empty one, as the call to renderer->firstTexBox() returns NULL (since the layout() still didn't happen) and no valid text is finally returned by webkit_accessible_text_get_name().

So, what I did to fix this is to follow what I see in many other tests in LayoutTests (which also depend on the page being properly loaded and layed-out first) and change your proposed test not to do anything until the page has completely been loaded (<body onload="startTest();">...</body>) and, on top of that, to add a timeout to ensure that there's enough time after that to make sure the test has been renderized. 

Moreover, I've added another timeout to ensure the js-test-post.js file has been loaded at the end of the test before calling to notifyDone, so the expected result and the actual one match perfectly.

Perhaps there's a better way to do this (not so many timeouts) but at this point this is the best way I found to fix this (which in the other hand seems to be a common practice in many tests) and I couldn't resist to share this findings now :-), hopefully to get some feedback as well in how to improve it (for instance, if it were possible to get notified somehow in the test when the layou() process has been done, and then to call to startTest(), it would be great as we'd avoid the timeout).

Looking forward to hearing some opinions, for the time being here you have the patch replacing your second one (just a new test).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list