[Webkit-unassigned] [Bug 45381] [Gtk] Adjust atk_text_get_text_at_offset to account for bullets/numbers in list items

[bugzilla-daemon at webkit.org]

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


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #67808|0                           |1
        is obsolete|                            |
  Attachment #68509|                            |review?
               Flag|                            |




--- Comment #6 from Mario Sanchez Prada <msanchez at igalia.com>  2010-09-23 05:07:29 PST ---
Created an attachment (id=68509)
 --> (https://bugs.webkit.org/attachment.cgi?id=68509)
Patch proposal + unit tes updated

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 67808 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=67808&action=review
> > 
> > i don't know how other people feel about adding a variable to RenderListMarker. sometimes they are quite picky about that. 
> 
> Well, that's the best way I found to allow returning the marker's suffix without having to recalculate it each time. But if there's a better way to do it, I'm more than happy to change it.

I finally found a simple way to expose the marker's suffix through a public function without having to add that variable to RenderListMarker, and it was just delegating on the internal static function listMarkerSuffix(), and then doing some small changes in the new funcion RenderListItem::markerTextWithSuffix(). And to be honest, I recognize I like it more this way, which hast the plus of not relying in the painting process to get the right value of the marker suffix.

> > need a layout test for this however
> 
> Wouldn't it be enough with an unit test to check the marker is what it's supposed to be?

I updated the unit test for ATK to ensure that the exposition of the marker is the right one. Guess this should be enough, but you (Chris) will better tell

> > > WebCore/ChangeLog:39
> > > +
> > 
> > indent comments when they are underneath the a filename. it makes it easier to read
> 
> Ok

Done.

> > > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1218
> > > +    RenderObject* renderer = static_cast<const AccessibilityRenderObject*>(object)->renderer();
> > 
> > check if renderer is nil as well
> 
> Ok.

Done.

> > > WebCore/rendering/RenderListItem.cpp:325
> > > +    const String& markerTxt = m_marker->text();
> > 
> > var name should be markerText
> 
> Ok.

Done.

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