Bug 58944 - bidiNext should be split into two separate functions
Summary: bidiNext should be split into two separate functions
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 59025 59033
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-19 16:53 PDT by Eric Seidel (no email)
Modified: 2011-04-26 16:05 PDT (History)
4 users (show)

See Also:


Attachments
Patch (12.45 KB, patch)
2011-04-20 18:30 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2011-04-19 16:53:21 PDT
bidiNext should be split into two separate functions

bidiNext has two (mostly unrelated) modes.

Either it's walking to the next RenderText, or it's walking to the next RenderInline.  This is controlled (confusingly) by the skipInlines bool, which when false, puts it in "walk to inlines" mode.

Here are the 7 callsites:

InlineIterator.h:             o = bidiNext(root, o, resolver, skipInlines); // Just plumbing
InlineIterator.h:         o = bidiNext(root, o, resolver, skipInlines); // Just plumbing

InlineIterator.h:     moveTo(bidiNext(m_root, m_obj, resolver), 0); // The normal walk-to-the-next  RenderText mode (and the only place that ever notifies the resolver when entering/leaving inlines).

InlineIterator.h:             obj = bidiNext(m_sor.root(), obj); // Used by InlineBidiIterator, really should just be an InlineIterator.increment() call.

RenderBlock.cpp:             o = bidiNext(this, o, 0, false, &endOfInline); // Wants all inlines
RenderBlockLineLayout.cpp:             o = bidiNext(this, o, 0, false, &endOfInline); // Wants all inlines

RenderBlockLineLayout.cpp:     RenderObject* next = bidiNext(block, o); // Looking for the next RenderText, probably should be an InlineIterator.increment() call.
RenderBlockLineLayout.cpp:         RenderObject* next = bidiNext(this, o); // Also probably should just be an InlineIterator.
Comment 1 Eric Seidel (no email) 2011-04-20 18:30:51 PDT
Created attachment 90467 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-04-20 18:31:36 PDT
Comment on attachment 90467 [details]
Patch

It may make the most sense to wait on landing this patch until after I've posted further re-writes and everyone can see where I'm going.  I'm pretty sure that the endOfInline nonsense can be avoided.