Bug 32069

Summary: Chromium: Need tickmarks in scrollbar
Product: WebKit Reporter: Avi Drissman <avi>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fishd, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch to implement tickmarks
none
Now with svn cp!
none
Patch to draw scrollbar ticks. Now with UX approval! none

Description Avi Drissman 2009-12-02 09:08:38 PST
Chromium draws tickmarks in the scrollbar when finding text. This needs to be done for the Mac.
Comment 1 Avi Drissman 2009-12-02 09:11:16 PST
Created attachment 44154 [details]
Patch to implement tickmarks

Note that in review, please compare ScrollbarThemeChromiumMac.* to ScrollbarThemeMac.*. It should be pretty self-explanatory.
Comment 2 WebKit Review Bot 2009-12-02 09:11:57 PST
style-queue ran check-webkit-style on attachment 44154 [details] without any errors.
Comment 3 Avi Drissman 2009-12-02 09:24:20 PST
Created attachment 44156 [details]
Now with svn cp!
Comment 4 WebKit Review Bot 2009-12-02 09:27:54 PST
style-queue ran check-webkit-style on attachment 44156 [details] without any errors.
Comment 5 Darin Fisher (:fishd, Google) 2009-12-02 09:35:21 PST
Comment on attachment 44156 [details]
Now with svn cp!

The code is all fine.  However, I'm concerned about the forking.

It seems like it would be nice to share code with ScrollbarThemeChromium.cpp
for drawing the tickmarks and to share code with ScrollbarThemeMac.mm for
much of the other bits.

-Darin
Comment 6 Darin Fisher (:fishd, Google) 2009-12-02 09:41:37 PST
Comment on attachment 44156 [details]
Now with svn cp!

Avi explained why it has to be this way for now.  We have forked RenderTheme similarly.
This will eventually become a painful thing to deal with, and I'm very interested in
finding a better solution.

But, for now r=me
Comment 7 WebKit Commit Bot 2009-12-02 11:03:31 PST
Comment on attachment 44156 [details]
Now with svn cp!

Clearing flags on attachment: 44156

Committed r51603: <http://trac.webkit.org/changeset/51603>
Comment 8 WebKit Commit Bot 2009-12-02 11:03:42 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Avi Drissman 2009-12-09 07:53:06 PST
Created attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!
Comment 10 Avi Drissman 2009-12-09 07:53:35 PST
Sigh. UX says we need to draw these prettier.
Comment 11 Darin Fisher (:fishd, Google) 2009-12-09 08:30:07 PST
Avi, does that last comment imply that this patch should no longer be up for review?
Comment 12 Avi Drissman 2009-12-09 08:35:43 PST
(In reply to comment #11)
> Avi, does that last comment imply that this patch should no longer be up for
> review?

Sorry. The latest patch, posted this morning, is what UX approved. It's up for review.
Comment 13 Darin Fisher (:fishd, Google) 2009-12-09 09:32:49 PST
Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

r=me
Comment 14 WebKit Commit Bot 2009-12-09 09:36:43 PST
Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

Rejecting patch 44538 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against bugzilla-tool.
Failed to run "['WebKitTools/Scripts/bugzilla-tool', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', '--quiet', '44538']" exit_code: 1
Last 500 characters of output:
nd_handle_errors
    self.run()
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/modules/landingsequence.py", line 54, in run
    self.clean()
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/modules/landingsequence.py", line 81, in clean
    step.run()
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/modules/buildsteps.py", line 101, in run
    os.chdir(self._tool._scm.checkout_root)
AttributeError: 'NoneType' object has no attribute 'checkout_root'
Comment 15 Avi Drissman 2009-12-09 11:20:19 PST
Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

Can we try committing again?
Comment 16 WebKit Commit Bot 2009-12-09 11:22:55 PST
Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

Rejecting patch 44538 from commit-queue.

avi@drissman.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py.

- If you have committer rights please correct the error in WebKitTools/Scripts/modules/committers.py by adding yourself to the file (no review needed) and then set the committer flag again.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.
Comment 17 Eric Seidel (no email) 2009-12-09 11:37:15 PST
Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

Sorry for the noise.  Adam believes to have fixed the issue.  I've restarted the commit bot.
Comment 18 WebKit Commit Bot 2009-12-09 11:52:36 PST
Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

Clearing flags on attachment: 44538

Committed r51909: <http://trac.webkit.org/changeset/51909>
Comment 19 WebKit Commit Bot 2009-12-09 11:52:42 PST
All reviewed patches have been landed.  Closing bug.