Re: RR, code review request: merge HyperlinkOverride into Hyperlink widget

Hello Ray :)

Would you take a look at this patch for the Hyperlink widget? It takes
the behavior from incubator's HyperlinkOverride widget and moves it
into the core Hyperlink.

This means that on a click event with a modifier key (with specific
keys counting, per-browser), we let the default browser action happen,
usually something like opening that link in a new tab.

I have a small doubt about this patch with regard to Chrome, but
please let me know what you think -- as far as I can tell, Chrome
doesn't do an "open in new tab" when you ctrl-click on a link to a URL
fragment (for example, our history tokens) -- but on the assumption
that this may change soon, this patch tries to do the behavior that
Chrome does for regular links, which is just like Firefox.

Patch is intended for the 1.6 release branch, r4214.

Thanks!

--
Alex Rudnick
swe, gwt, atl

[rjrjr, 2008-12-02 17:50:04]

This looks great to me, Alex. And I think you're doing as right a thing as you
can WRT Chrome. Just a couple of nits noted below.

http://codereview.appspot.com/8696/diff/1/4
File user/src/com/google/gwt/user/client/ui/impl/HyperlinkImplIE.java (right):

http://codereview.appspot.com/8696/diff/1/4#newcode31
Line 31: ctrlisModifier = (getInternetExplorerVersion() >= 7);
Why the static block? Seems like you can just do this inline.

http://codereview.appspot.com/8696/diff/1/5
File user/src/com/google/gwt/user/client/ui/impl/HyperlinkImplSafari.java
(right):

http://codereview.appspot.com/8696/diff/1/5#newcode31
Line 31: shiftIsModifier = onChrome();
again, don't see the need for the block.