Bug 32069 – Chromium: Need tickmarks in scrollbar

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Avi Drissman]

Chromium draws tickmarks in the scrollbar when finding text. This needs to be done for the Mac.

[Avi Drissman]

Created attachment 44154 [details]
Patch to implement tickmarks

Note that in review, please compare ScrollbarThemeChromiumMac.* to ScrollbarThemeMac.*. It should be pretty self-explanatory.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44154 [details] without any errors.

[Avi Drissman]

Created attachment 44156 [details]
Now with svn cp!

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44156 [details] without any errors.

[Darin Fisher (:fishd, Google)]

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

[Darin Fisher (:fishd, Google)]

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

[WebKit Commit Bot]

Comment on attachment 44156 [details]
Now with svn cp!

Clearing flags on attachment: 44156

Committed r51603: <http://trac.webkit.org/changeset/51603>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Avi Drissman]

Created attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

[Avi Drissman]

Sigh. UX says we need to draw these prettier.

[Darin Fisher (:fishd, Google)]

Avi, does that last comment imply that this patch should no longer be up for review?

[Avi Drissman]

(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.

[Darin Fisher (:fishd, Google)]

Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

r=me

[WebKit Commit Bot]

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'

[Avi Drissman]

Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

Can we try committing again?

[WebKit Commit Bot]

Comment on attachment 44538 [details]
Patch to draw scrollbar ticks. Now with UX approval!

Rejecting patch 44538 from commit-queue.

[e-mail] 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.

[Eric Seidel (no email)]

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.

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.