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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 9265020: [uber] make the navigation controls an iframe (Closed)

Created:
8 years, 11 months ago by Evan Stade
Modified:
8 years, 11 months ago
Reviewers:
csilv, Dan Beam
CC:
chromium-reviews, Aaron Boodman, arv (Not doing code reviews), mihaip+watch_chromium.org
Visibility:
Public.

Description

[uber] make the navigation controls an iframe this allows us to control the z-index of the nav controls relative to the iframes (iframes are always on top, but you can control the relative order of iframes compared to other iframes) At the cost of increased code complexity, this allows us to remove the overlay hack, and more importantly paves the way for overlapping content over the nav bar, which has suddenly become absolutely necessary for subpages to do. BUG=110658 TEST=manual. History still works. Clicking the nav buttons still works. if you resize the browser window really small while showing an overlay it will use up all available space. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=118596

Patch Set 1 #

Total comments: 28

Patch Set 2 : revert overlay change so extensions can use it #

Total comments: 2

Patch Set 3 : opt = #

Unified diffs Side-by-side diffs Delta from patch set Stats (+337 lines, -156 lines) Patch
M chrome/browser/browser_resources.grd View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/resources/about_page/about_page.css View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/about_page/about_page.html View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/extensions/extensions.css View 2 chunks +1 line, -5 lines 0 comments Download
M chrome/browser/resources/extensions/extensions.html View 2 chunks +3 lines, -5 lines 0 comments Download
M chrome/browser/resources/options2/options.html View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/options2/options_page.css View 1 2 chunks +9 lines, -6 lines 0 comments Download
M chrome/browser/resources/options2/options_page.js View 9 chunks +12 lines, -22 lines 0 comments Download
M chrome/browser/resources/shared/css/chrome_shared2.css View 1 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/resources/shared/js/jstemplate_compiled.js View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/resources/uber/uber.css View 3 chunks +14 lines, -47 lines 0 comments Download
M chrome/browser/resources/uber/uber.html View 1 2 1 chunk +2 lines, -15 lines 0 comments Download
M chrome/browser/resources/uber/uber.js View 1 6 chunks +51 lines, -40 lines 0 comments Download
A chrome/browser/resources/uber/uber_frame.css View 1 chunk +40 lines, -0 lines 0 comments Download
A chrome/browser/resources/uber/uber_frame.html View 1 chunk +32 lines, -0 lines 0 comments Download
A chrome/browser/resources/uber/uber_frame.js View 1 chunk +87 lines, -0 lines 0 comments Download
M chrome/browser/resources/uber/uber_utils.js View 1 2 1 chunk +22 lines, -9 lines 0 comments Download
M chrome/browser/ui/webui/chrome_web_ui_factory.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/uber/uber_ui.h View 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/uber/uber_ui.cc View 4 chunks +37 lines, -1 line 0 comments Download
M chrome/common/url_constants.h View 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/common/url_constants.cc View 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
Evan Stade
YOU get a frame! and YOU get a frame!
8 years, 11 months ago (2012-01-19 23:58:03 UTC) #1
Dan Beam
http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css File chrome/browser/resources/options2/options_page.css (right): http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css#newcode148 chrome/browser/resources/options2/options_page.css:148: bottom: 0; confused, why bottom & top? some magic ...
8 years, 11 months ago (2012-01-20 00:59:02 UTC) #2
Dan Beam
On 2012/01/19 23:58:03, Evan Stade wrote: > YOU get a frame! and YOU get a ...
8 years, 11 months ago (2012-01-20 00:59:13 UTC) #3
Evan Stade
http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/uber/uber.js File chrome/browser/resources/uber/uber.js (right): http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/uber/uber.js#newcode39 chrome/browser/resources/uber/uber.js:39: console.log('removing'); On 2012/01/20 00:59:02, Dan Beam wrote: > remove ...
8 years, 11 months ago (2012-01-20 03:27:07 UTC) #4
csilv
http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css File chrome/browser/resources/options2/options_page.css (right): http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css#newcode142 chrome/browser/resources/options2/options_page.css:142: #overlay { This seems useful to other web-ui, why ...
8 years, 11 months ago (2012-01-20 03:30:11 UTC) #5
Evan Stade
http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css File chrome/browser/resources/options2/options_page.css (right): http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css#newcode142 chrome/browser/resources/options2/options_page.css:142: #overlay { On 2012/01/20 03:30:11, csilv wrote: > This ...
8 years, 11 months ago (2012-01-20 03:36:07 UTC) #6
csilv
http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css File chrome/browser/resources/options2/options_page.css (right): http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css#newcode142 chrome/browser/resources/options2/options_page.css:142: #overlay { On 2012/01/20 03:36:07, Evan Stade wrote: > ...
8 years, 11 months ago (2012-01-20 03:55:59 UTC) #7
Evan Stade
On 2012/01/20 03:55:59, csilv wrote: > http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css > File chrome/browser/resources/options2/options_page.css (right): > > http://codereview.chromium.org/9265020/diff/1/chrome/browser/resources/options2/options_page.css#newcode142 > ...
8 years, 11 months ago (2012-01-20 04:09:50 UTC) #8
csilv
lgtm
8 years, 11 months ago (2012-01-20 04:38:05 UTC) #9
Dan Beam
lgtm with note http://codereview.chromium.org/9265020/diff/7002/chrome/browser/resources/uber/uber_utils.js File chrome/browser/resources/uber/uber_utils.js (right): http://codereview.chromium.org/9265020/diff/7002/chrome/browser/resources/uber/uber_utils.js#newcode15 chrome/browser/resources/uber/uber_utils.js:15: * @param {Object} opt_params Optional property ...
8 years, 11 months ago (2012-01-20 06:35:25 UTC) #10
Evan Stade
http://codereview.chromium.org/9265020/diff/7002/chrome/browser/resources/uber/uber_utils.js File chrome/browser/resources/uber/uber_utils.js (right): http://codereview.chromium.org/9265020/diff/7002/chrome/browser/resources/uber/uber_utils.js#newcode15 chrome/browser/resources/uber/uber_utils.js:15: * @param {Object} opt_params Optional property bag of parameters ...
8 years, 11 months ago (2012-01-20 21:40:38 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/estade@chromium.org/9265020/11001
8 years, 11 months ago (2012-01-20 21:41:53 UTC) #12
commit-bot: I haz the power
8 years, 11 months ago (2012-01-20 21:41:56 UTC) #13 
Can't process patch for file
chrome/browser/resources/shared/js/jstemplate_compiled.js.
Hunk header is incorrect: -1 vs -2

Powered by Google App Engine
This is Rietveld 408576698