Bug 57178 – InlineBox::prevOnline and InlineBox::prevOnlineExists() are confusing and should be renamed

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Eric Seidel (no email)]

InlineBox::prevOnline and InlineBox::prevOnlineExists() are confusing and should be renamed

prevOnline() just returns m_prev and is probably named with "onLine" to indicate that it's on the same line (and not a prev/next in the sense of a RenderLineBoxList).

prevOnlineExists() answers the question of if there is a previous line box on the same line, not necessarily within the current InlineFlowBox container.

For example, the following code is wrong:

prevOnlineExists() && prevOnline()->foo()

One of these should be renamed to prevent confusion.

[Eric Seidel (no email)]

This came up when Yael and I were looking at fixing bug 9272.

[Yael]

After http://trac.webkit.org/changeset/82419, prevOnLineExists() and nextOnLineExists() are no longer used. I am preparing a patch to remove them, and all the flags associated with them.

[Yael]

Created attachment 89063 [details]
Patch.

Remove nextOnLineExists() and prevOnLineExists(), as they are not used anymore.

[Eric Seidel (no email)]

Comment on attachment 89063 [details]
Patch.

OK.  Sounds great. Thanks!

[WebKit Commit Bot]

Comment on attachment 89063 [details]
Patch.

Clearing flags on attachment: 89063

Committed r83515: <http://trac.webkit.org/changeset/83515>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

This broke GTK builds. They were apparently used in AccessibilityObjectWrapperAtk.cpp:
http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/21347/steps/compile-webkit/logs/stdio

[Eric Seidel (no email)]

Wow, that's so wrong it hurts.

[Ryosuke Niwa]

Committed r83540: <http://trac.webkit.org/changeset/83540>

[Ryosuke Niwa]

Sorry, I had to roll out the patch in http://trac.webkit.org/changeset/83540 because there wasn't obvious alternatives to call.

[Yael]

(In reply to comment #10)
> Sorry, I had to roll out the patch in http://trac.webkit.org/changeset/83540 because there wasn't obvious alternatives to call.

Thank you for rolling out. Sorry I was not online to see that it broke the build.

[Ahmad Saleem]

Do we need to track this?

This is in Legacy Line Layout. Only following references exists in our code besides in 'LegacyInlineBox.cpp/h':

In RenderLineBreak:

isLastOnLine = !containingBlock->containingBlock()->inlineBoxWrapper()->nextOnLineExists();

isLastOnLine = !containingBlock->inlineBoxWrapper()->nextOnLineExists();

In RenderText:

isLastOnLine = !containingBlock->containingBlock()->inlineBoxWrapper()->nextOnLineExists();

isLastOnLine = !containingBlock->inlineBoxWrapper()->nextOnLineExists();

[zalan]

(In reply to Ahmad Saleem from comment #12)
> Do we need to track this?
Certainly not. This code is soon to be completely removed.