Add facility to fix about: and file: links from chrome:// pages.

Add facility to fix about: and file: links from chrome:// pages.

Currently, about: links all go to about:blank. This fixes that for left clicks and middle clicks, although context menu navigations (e.g. open in new tab) still won't work.

This also "fixes" file: links in the same way. Currently, file links already work as is, but after we make --new-chrome-ui-security-model the default, they will cease to work without this fix. Context menu navigations will, however, continue to work.

The fix is currently applied to the history page and one link in settings.

BUG=69140
TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72401

[Evan Stade, 2011-01-21 22:33:29]

My hope is that this will allow us to land
http://codereview.chromium.org/6266017/ , which sets the new chrome ui security
model as the default.

[abarth-chromium, 2011-01-21 22:48:00]

Looks plausible to me, but I'm not an expert here.

[arv (Not doing code reviews), 2011-01-21 23:07:32]

I think this approach is a fine workaround.

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/generi...
File chrome/browser/dom_ui/generic_handler.cc (right):

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/generi...
chrome/browser/dom_ui/generic_handler.cc:29: WindowOpenDisposition disposition;
Can you also include things like shiftKey, ctrlKey and metaKey?

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/generi...
File chrome/browser/dom_ui/generic_handler.h (right):

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/generi...
chrome/browser/dom_ui/generic_handler.h:13: class GenericHandler : public
DOMMessageHandler {
I would call this NavigationHandler

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/options_ui.cc (right):

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/options_ui.cc:217: // eventually delete them. Skip
over the generic handler.
Why?

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/resources/his...
File chrome/browser/resources/history.html (right):

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/resources/his...
chrome/browser/resources/history.html:167: handleFileOrAboutLinkClicks(link);
I don't like adding this to every link... more later.

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/resources/sha...
File chrome/browser/resources/shared/js/util.js (right):

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/resources/sha...
chrome/browser/resources/shared/js/util.js:75: function
handleFileOrAboutLinkClicks(link) {
Please use a global handler instead.

function handleLinkClickOrMouseUp(e) {
  var el = e.target;
  if (el.nodeType == Node.ELEMENT_NODE &&
      el.webkitMatchesSelector('A, A *')) {
    while (el.tagName != 'A') {
      el = el.parentElement;
    }
    if (el.protocol == 'file:' || el.protocol == 'about:') {
      chrome.send('navigateToUrl', [el.href, e.button, e.shiftKey, e.ctrlKey,
          e.altKey, e.metaKey]);
      e.preventDefault();
    }
  }
}

document.addEventListener('click', handleLinkClickOrMouseUp, true);
document.addEventListener('mouseup', handleLinkClickOrMouseUp, true);


This way we do not need to manually call handleFileOrAboutLinkClicks on the
links we care about. This also ensures we do not duplicate the code.

[Evan Stade, 2011-01-21 23:41:34]

what does the meta key do?

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/generi...
File chrome/browser/dom_ui/generic_handler.h (right):

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/generi...
chrome/browser/dom_ui/generic_handler.h:13: class GenericHandler : public
DOMMessageHandler {
On 2011/01/21 23:07:32, arv wrote:
> I would call this NavigationHandler

I forgot to add a comment to the class, but the idea was that general utilities
would all be added here.

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/options_ui.cc (right):

http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/options_ui.cc:217: // eventually delete them. Skip
over the generic handler.
On 2011/01/21 23:07:32, arv wrote:
> Why?

uninitialize is a function of OptionsPageUIHandler, not general dom handlers

[arv (Not doing code reviews), 2011-01-21 23:51:10]

On Fri, Jan 21, 2011 at 15:41,  <estade@chromium.org> wrote:
> what does the meta key do?

meta == command (open apple or whatevs) and is used to save a link or
something. I don't remember and my mac doesn;t work any more.

>
>
http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/generi...
> File chrome/browser/dom_ui/generic_handler.h (right):
>
>
http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/generi...
> chrome/browser/dom_ui/generic_handler.h:13: class GenericHandler :
> public DOMMessageHandler {
> On 2011/01/21 23:07:32, arv wrote:
>>
>> I would call this NavigationHandler
>
> I forgot to add a comment to the class, but the idea was that general
> utilities would all be added here.
>
>
http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/option...
> File chrome/browser/dom_ui/options/options_ui.cc (right):
>
>
http://codereview.chromium.org/6269015/diff/3001/chrome/browser/dom_ui/option...
> chrome/browser/dom_ui/options/options_ui.cc:217: // eventually delete
> them. Skip over the generic handler.
> On 2011/01/21 23:07:32, arv wrote:
>>
>> Why?
>
> uninitialize is a function of OptionsPageUIHandler, not general dom
> handlers
>
> http://codereview.chromium.org/6269015/
>

[Evan Stade, 2011-01-22 01:26:10]

implemented your suggestion.

[Evan Stade, 2011-01-22 01:27:29]

btw, I didn't implement meta, because all 3 event_utils (e.g.,
browser/ui/views/event_utils.cc) don't respect the meta key.

[arv (Not doing code reviews), 2011-01-24 19:18:07]

LGTM