Bug 57178

Summary: InlineBox::prevOnline and InlineBox::prevOnlineExists() are confusing and should be renamed
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: ahmad.saleem792, commit-queue, hyatt, mitz, rniwa, simon.fraser, xji, yael, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch. none

Description Eric Seidel (no email) 2011-03-27 04:03:33 PDT
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.
Comment 1 Eric Seidel (no email) 2011-03-27 04:21:23 PDT
This came up when Yael and I were looking at fixing bug 9272.
Comment 2 Yael 2011-04-11 12:45:24 PDT
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.
Comment 3 Yael 2011-04-11 13:13:08 PDT
Created attachment 89063 [details]
Patch.

Remove nextOnLineExists() and prevOnLineExists(), as they are not used anymore.
Comment 4 Eric Seidel (no email) 2011-04-11 13:19:27 PDT
Comment on attachment 89063 [details]
Patch.

OK.  Sounds great. Thanks!
Comment 5 WebKit Commit Bot 2011-04-11 15:18:21 PDT
Comment on attachment 89063 [details]
Patch.

Clearing flags on attachment: 89063

Committed r83515: <http://trac.webkit.org/changeset/83515>
Comment 6 WebKit Commit Bot 2011-04-11 15:18:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Ryosuke Niwa 2011-04-11 16:55:53 PDT
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
Comment 8 Eric Seidel (no email) 2011-04-11 17:12:56 PDT
Wow, that's so wrong it hurts.
Comment 9 Ryosuke Niwa 2011-04-11 17:16:14 PDT
Committed r83540: <http://trac.webkit.org/changeset/83540>
Comment 10 Ryosuke Niwa 2011-04-11 17:18:49 PDT
Sorry, I had to roll out the patch in http://trac.webkit.org/changeset/83540 because there wasn't obvious alternatives to call.
Comment 11 Yael 2011-04-11 17:42:46 PDT
(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.
Comment 12 Ahmad Saleem 2023-06-03 05:31:03 PDT
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();
Comment 13 zalan 2023-06-03 06:09:00 PDT
(In reply to Ahmad Saleem from comment #12)
> Do we need to track this?
Certainly not. This code is soon to be completely removed.