Bug 28125 - [Haiku] Adding ScrollbarTheme to WebCore.
Summary: [Haiku] Adding ScrollbarTheme to WebCore.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-09 05:23 PDT by Maxime Simon
Modified: 2009-08-12 13:23 PDT (History)
2 users (show)

See Also:


Attachments
Patch to add the ScrollbarTheme files for Haiku. (10.42 KB, patch)
2009-08-09 05:28 PDT, Maxime Simon
eric: review+
Details | Formatted Diff | Diff
Patch to add the ScrollbarTheme files for Haiku. (10.76 KB, patch)
2009-08-11 05:27 PDT, Maxime Simon
no flags Details | Formatted Diff | Diff
Patch to add the ScrollbarTheme files for Haiku. (10.76 KB, patch)
2009-08-11 09:12 PDT, Maxime Simon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maxime Simon 2009-08-09 05:23:21 PDT
Patch to add the ScrollbarThemeHaiku.* files to WebCore.
Comment 1 Maxime Simon 2009-08-09 05:28:42 PDT
Created attachment 34410 [details]
Patch to add the ScrollbarTheme files for Haiku.
Comment 2 Eric Seidel (no email) 2009-08-09 08:00:41 PDT
Comment on attachment 34410 [details]
Patch to add the ScrollbarTheme files for Haiku.

I would have written:
         return IntRect(scrollbar->x(), scrollbar->y(),
 77                        scrollbar->width() < 2 * thickness ? scrollbar->width() / 2 : thickness, thickness);
 78     return IntRect(scrollbar->x(), scrollbar->y(),
 79                    thickness, scrollbar->height() < 2 * thickness ? scrollbar->height() / 2 : thickness);

As:

int buttonWidth(int scrollbarWidth, int thickness)
{
      return scrollbarWidth < 2 * thickness ? scrollbarWidth / 2 : thickness;
}

IntPoint buttonOrigin(scrollbar->x(), scrollbar->y());
IntSize buttonSize = scrollbar->orientation() == HorizontalScrollbar
                                  ? IntSize(buttonWidth(scrollbar->width()), thickness)
                                  : IntSize(thickness, buttonWidth(scrollbar->width());
return IntRect(buttonOrigin, buttonSize);

That way you can re-use buttonWidth() on the next function too.

The code looks OK as is though.
Comment 3 Ryan Leavengood 2009-08-10 19:44:45 PDT
Maxime,

You should also add yourself above Apple in the copyright section of this code (and other code you have written or modified.)

Though since Eric alread r+ this, you can make some new patches later to fix up the copyrights. Or consider also making the change Eric suggested and make a new patch. It's up to you ;)
Comment 4 Maxime Simon 2009-08-11 05:27:46 PDT
Created attachment 34550 [details]
Patch to add the ScrollbarTheme files for Haiku.

Make some changes as suggested by Eric and Ryan, even if the previous patches was r+. I also made some style cleanup.
Comment 5 Maxime Simon 2009-08-11 09:12:37 PDT
Created attachment 34564 [details]
Patch to add the ScrollbarTheme files for Haiku.

Sorry, the previous had a mistake. I changed scrollbar->height() to scrollbar->width(), so it didn't display correctly the back button.
Comment 6 Eric Seidel (no email) 2009-08-12 10:35:15 PDT
Comment on attachment 34564 [details]
Patch to add the ScrollbarTheme files for Haiku.

I wouldn't have bothered to create this variable:

83     IntRect buttonRect(buttonOrigin, buttonSize);
 84 
 85     return buttonRect;

Looks fine though.
Comment 7 Eric Seidel (no email) 2009-08-12 13:23:19 PDT
Comment on attachment 34564 [details]
Patch to add the ScrollbarTheme files for Haiku.

Clearing flags on attachment: 34564

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	A	WebCore/platform/haiku/ScrollbarThemeHaiku.cpp
	A	WebCore/platform/haiku/ScrollbarThemeHaiku.h
Committed r47143
	M	WebCore/ChangeLog
	A	WebCore/platform/haiku/ScrollbarThemeHaiku.cpp
	A	WebCore/platform/haiku/ScrollbarThemeHaiku.h
r47143 = 1e5acb63da2fefd1e799f6c31987d9899a34e057 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/47143
Comment 8 Eric Seidel (no email) 2009-08-12 13:23:22 PDT
All reviewed patches have been landed.  Closing bug.