REGRESSION: A history item should not be checked when the bookmark star is clicked.

REGRESSION: A history item should not be checked when the bookmark star is clicked.

This is a regression by r212474.
preventDefault() is called for a click event on the bookmark star,
However we manually check the checkbox by entryBox click and ignore the
|defaultPrevented| state.  We should repect it.

BUG=262869
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213351

[tkent, 2013-07-23 06:59:33]

arv, would you review this please?

[Patrick Dubroy, 2013-07-23 10:16:34]

https://codereview.chromium.org/19771022/diff/1/chrome/browser/resources/hist...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/19771022/diff/1/chrome/browser/resources/hist...
chrome/browser/resources/history/history.js:1775: if (event.defaultPrevented)
I think it makes more sense to use event.stopPropagation() here:
http://goo.gl/Eu7gvh.

[arv (Not doing code reviews), 2013-07-23 17:39:18]

LGTM

https://codereview.chromium.org/19771022/diff/1/chrome/browser/resources/hist...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/19771022/diff/1/chrome/browser/resources/hist...
chrome/browser/resources/history/history.js:1775: if (event.defaultPrevented)
This needs a comment.

https://codereview.chromium.org/19771022/diff/1/chrome/browser/resources/hist...
chrome/browser/resources/history/history.js:1775: if (event.defaultPrevented)
On 2013/07/23 10:16:35, dubroy wrote:
> I think it makes more sense to use event.stopPropagation() here:
> http://goo.gl/Eu7gvh.

I prefer to not call stopPropagation unless needed. It does have unexpected side
effects.

[tkent, 2013-07-24 00:34:39]

https://codereview.chromium.org/19771022/diff/1/chrome/browser/resources/hist...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/19771022/diff/1/chrome/browser/resources/hist...
chrome/browser/resources/history/history.js:1775: if (event.defaultPrevented)
On 2013/07/23 10:16:35, dubroy wrote:
> I think it makes more sense to use event.stopPropagation() here:
> http://goo.gl/Eu7gvh.

It also fixes this bug.  However I'd like to apply this approach in order to
centralize the hack.  Click handler for the star button should not know this
special handling of entryBox click.

https://codereview.chromium.org/19771022/diff/1/chrome/browser/resources/hist...
chrome/browser/resources/history/history.js:1775: if (event.defaultPrevented)
On 2013/07/23 17:39:19, arv wrote:
> This needs a comment.

will do.

[commit-bot: I haz the power, 2013-07-24 00:42:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tkent@chromium.org/19771022/12001

[commit-bot: I haz the power, 2013-07-24 05:16:19]

Change committed as 213351