Bug 45687 - [GTK] Code simplification needed in Atk Wrapper
Summary: [GTK] Code simplification needed in Atk Wrapper
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-13 11:01 PDT by Mario Sanchez Prada
Modified: 2010-09-13 20:16 PDT (History)
2 users (show)

See Also:


Attachments
Patch to fix this issue (3.15 KB, patch)
2010-09-13 11:09 PDT, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2010-09-13 11:01:50 PDT
The following piece of code in WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp should be simplified:

    // Set preliminar values for start and end offsets
    startOffset = nodeRangeStart.offsetInContainerNode();
    endOffset = nodeRangeEnd.offsetInContainerNode();

    // If the end node is different then the start node, iterate over
    // those among them to build the effective value for endOffset
    if (nodeRangeStart.anchorNode() != nodeRangeEnd.anchorNode()) {
        RefPtr<Range> nodeRange = Range::create(node->document(), nodeRangeStart, positionBeforeNode(nodeRangeEnd.anchorNode()));
        for (TextIterator it(nodeRange.get()); !it.atEnd(); it.advance()) {
            RefPtr<Range> range = it.range();
            if (range->startContainer()->isTextNode())
                endOffset += range->endOffset();
        }
    }


This could be simplified using TextIterator::rangeLength(), making the code cleaner and more readable
Comment 1 Mario Sanchez Prada 2010-09-13 11:09:08 PDT
Created attachment 67433 [details]
Patch to fix this issue

Attaching a patch to fix this "issue" by simplifying the code as expected:

  Use TextIterator::rangeLength() to calculate endOffset

     * accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:
     (getSelectionOffsetsForObject): Don't manually iterate over a
     artificial new range but just call TextIterator::rangeLength on
     the range created between the start and end positions.

Hence, asking for review (and for the cq+ flag as well ;-)
Comment 2 Martin Robinson 2010-09-13 11:16:53 PDT
Comment on attachment 67433 [details]
Patch to fix this issue

Awesome. Thanks!
Comment 3 WebKit Commit Bot 2010-09-13 20:16:03 PDT
Comment on attachment 67433 [details]
Patch to fix this issue

Clearing flags on attachment: 67433

Committed r67435: <http://trac.webkit.org/changeset/67435>
Comment 4 WebKit Commit Bot 2010-09-13 20:16:08 PDT
All reviewed patches have been landed.  Closing bug.