Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(401)

Issue 155067: Hookup Print HTML page to the DOM UI for Print Preview and Settings... (Closed)

Created:
11 years, 5 months ago by Mohamed Mansour
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com, Ben Goodger (Google)
Visibility:
Public.

Description

Hookup Print HTML page to the DOM UI for Print Preview and Settings Depends on initial foundation: http://src.chromium.org/viewvc/chrome?view=rev&revision=19906 And html mockup: http://src.chromium.org/viewvc/chrome?view=rev&revision=19918 BUG=173, 947 TEST=The user will see the print html page as a html test webpage. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=20595

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 2

Patch Set 4 : '' #

Patch Set 5 : '' #

Total comments: 1

Patch Set 6 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+101 lines, -8 lines) Patch
M chrome/browser/child_process_security_policy.cc View 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/dom_ui/chrome_url_data_manager.cc View 3 4 5 3 chunks +15 lines, -5 lines 0 comments Download
M chrome/browser/dom_ui/dom_ui_factory.cc View 3 4 5 2 chunks +5 lines, -1 line 0 comments Download
M chrome/browser/dom_ui/print_ui.h View 1 2 3 4 5 2 chunks +19 lines, -0 lines 0 comments Download
M chrome/browser/dom_ui/print_ui.cc View 1 2 3 4 5 2 chunks +52 lines, -1 line 0 comments Download
M chrome/browser/resources/print_tab.html View 1 2 3 4 5 2 chunks +2 lines, -1 line 0 comments Download
M chrome/renderer/render_thread.cc View 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Mohamed Mansour
This is the implementation of DOMUI. Still, I can't figure out how to connect all ...
11 years, 5 months ago (2009-07-07 23:51:49 UTC) #1
Mohamed Mansour
We can't use this approach creating a DOMUI since we are using a print: scheme ...
11 years, 5 months ago (2009-07-08 03:52:33 UTC) #2
Mohamed Mansour
^^ I meant reimplementing this dude: chrome/browser/dom_ui/chrome_url_data_manager.h to be something like: chrome/browser/dom_ui/print_url_data_manager.h Its going to ...
11 years, 5 months ago (2009-07-08 03:53:51 UTC) #3
M-A Ruel
I'm definitely not the best reviewer for this part. Could you find someone from the ...
11 years, 5 months ago (2009-07-08 14:39:36 UTC) #4
Mohamed Mansour
Here is the screenshot: http://i29.tinypic.com/2hzihol.jpg
11 years, 5 months ago (2009-07-09 03:03:32 UTC) #5
Mohamed Mansour
It is all connected.
11 years, 5 months ago (2009-07-09 03:03:44 UTC) #6
M-A Ruel
http://codereview.chromium.org/155067/diff/1012/15 File chrome/browser/child_process_security_policy.cc (right): http://codereview.chromium.org/155067/diff/1012/15#newcode234 Line 234: I'm not fine with adding a partially implemented ...
11 years, 5 months ago (2009-07-09 19:21:27 UTC) #7
Mohamed Mansour
Done, correct, the best place to put the ifdef would be in the dom_ui_factory since ...
11 years, 5 months ago (2009-07-10 01:53:54 UTC) #8
Mohamed Mansour
Glen: Is everything good? I would like to commit this so I could continue and ...
11 years, 5 months ago (2009-07-13 16:41:47 UTC) #9
Glen Murphy
11 years, 5 months ago (2009-07-14 01:42:42 UTC) #10
LGTM with one comment:

http://codereview.chromium.org/155067/diff/1030/31
File chrome/browser/dom_ui/chrome_url_data_manager.cc (right):

http://codereview.chromium.org/155067/diff/1030/31#newcode144
Line 144: DCHECK(url.SchemeIs(kChromeURLScheme) ||
url.SchemeIs(chrome::kPrintScheme));
Not sure this should be in the else - this scheme should work for
PERSONALIZATION users, too.

Powered by Google App Engine
This is Rietveld 408576698