Register Chrome as MHTML Viewer on Mac OS X.

Register Chrome as MHTML Viewer on Mac OS X.

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

[benjhayden, 2014-07-21 18:14:28]

Chrome can save pages as MHTML with chrome://flags/#save-page-as-mhtml
It should recommend to the OS that it can also view them.

[Mark Mentovai, 2014-07-21 18:32:17]

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist
File chrome/app/app-Info.plist (right):

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist#ne...
chrome/app/app-Info.plist:55: <string>mhtml</string>
Your CL description says something about a flag. Can you confirm that Chrome is
able to open MHTML files even when that flag is not present?

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist#ne...
chrome/app/app-Info.plist:55: <string>mhtml</string>
xhtml and one or two others are maybe in a bad spot, but this list is generally
sorted by the primary extension. You should move this entry to be between jpg
and ogg.

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist#ne...
chrome/app/app-Info.plist:62: <string>multipart/related</string>
http://en.wikipedia.org/wiki/MHTML shows two more MIME types. Should those be
listed too?

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist#ne...
chrome/app/app-Info.plist:66: <key>CFBundleTypeOSTypes</key>
Don’t specify this at all. An OSType is a 32-bit integer normally represented as
four ASCII characters. It relates to the classic Mac file type code that’s set
for the file. MHTML is not a valid OS file type code, and it’s likely that there
is no code that’s properly used for this file type, so you should just leave
this key and array out entirely.

[benjhayden, 2014-07-21 18:41:49]

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist
File chrome/app/app-Info.plist (right):

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist#ne...
chrome/app/app-Info.plist:55: <string>mhtml</string>
On 2014/07/21 18:32:16, Mark Mentovai wrote:
> xhtml and one or two others are maybe in a bad spot, but this list is
generally
> sorted by the primary extension. You should move this entry to be between jpg
> and ogg.

Done.

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist#ne...
chrome/app/app-Info.plist:55: <string>mhtml</string>
On 2014/07/21 18:32:16, Mark Mentovai wrote:
> Your CL description says something about a flag. Can you confirm that Chrome
is
> able to open MHTML files even when that flag is not present?

The flag only controls whether chrome saves the page as mhtml, not whether
chrome can open mhtml files.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/dow...
Chrome should be able to open mhtml files regardless of that flag, or any other
afaik.

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist#ne...
chrome/app/app-Info.plist:62: <string>multipart/related</string>
On 2014/07/21 18:32:16, Mark Mentovai wrote:
> http://en.wikipedia.org/wiki/MHTML shows two more MIME types. Should those be
> listed too?

Done.

https://codereview.chromium.org/409543005/diff/1/chrome/app/app-Info.plist#ne...
chrome/app/app-Info.plist:66: <key>CFBundleTypeOSTypes</key>
On 2014/07/21 18:32:16, Mark Mentovai wrote:
> Don’t specify this at all. An OSType is a 32-bit integer normally represented
as
> four ASCII characters. It relates to the classic Mac file type code that’s set
> for the file. MHTML is not a valid OS file type code, and it’s likely that
there
> is no code that’s properly used for this file type, so you should just leave
> this key and array out entirely.

Done.

[Mark Mentovai, 2014-07-21 18:43:52]

LGTM

[benjhayden, 2014-07-21 20:41:49]

The CQ bit was checked by benjhayden@chromium.org

[commit-bot: I haz the power, 2014-07-21 20:42:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/benjhayden@chromium.org/409543005/20001

[commit-bot: I haz the power, 2014-07-21 22:45:02]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-07-21 22:49:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-21 22:49:57]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[benjhayden, 2014-07-21 23:28:18]

cpu: ownership rubberstamp?

[cpu_(ooo_6.6-7.5), 2014-07-22 01:31:53]

lgtm

[benjhayden, 2014-07-22 02:40:45]

The CQ bit was checked by benjhayden@chromium.org

[commit-bot: I haz the power, 2014-07-22 02:41:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/benjhayden@chromium.org/409543005/20001

[commit-bot: I haz the power, 2014-07-22 06:12:34]

Change committed as 284636