Bug 57178 - InlineBox::prevOnline and InlineBox::prevOnlineExists() are confusing and should be renamed
Summary: InlineBox::prevOnline and InlineBox::prevOnlineExists() are confusing and sho...
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-27 04:03 PDT by Eric Seidel (no email)
Modified: 2011-04-11 17:42 PDT (History)
6 users (show)

See Also:


Attachments
Patch. (3.65 KB, patch)
2011-04-11 13:13 PDT, Yael
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-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.