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

Issue 14812: Duplicate popup window should have same size and scroll ... (Closed)

Created:
12 years ago by Mohamed Mansour (USE mhm)
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Duplicate popup window should have same size and scroll position as original popup. BUG=5632 (http://crbug.com/5632) Patch by Mohamed Mansour.

Patch Set 1 #

Total comments: 1

Patch Set 2 : '' #

Total comments: 1

Patch Set 3 : '' #

Total comments: 1

Patch Set 4 : '' #

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -0 lines) Patch
M chrome/browser/browser.cc View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
Mohamed Mansour (USE mhm)
12 years ago (2008-12-17 22:32:04 UTC) #1
Mohamed Mansour (USE mhm)
added Elliot for review since Ben is on vacation
12 years ago (2008-12-17 22:41:59 UTC) #2
Peter Kasting
http://codereview.chromium.org/14812/diff/1/2 File chrome/browser/browser.cc (right): http://codereview.chromium.org/14812/diff/1/2#newcode1332 Line 1332: // The new popup window should have the ...
12 years ago (2008-12-17 22:47:46 UTC) #3
Mohamed Mansour (USE mhm)
> * Wouldn't we want this for app windows too? Normal windows duplicate it by ...
12 years ago (2008-12-17 22:57:55 UTC) #4
Peter Kasting
On 2008/12/17 22:57:55, Mohamed Mansour wrote: > > * Wouldn't we want this for app ...
12 years ago (2008-12-17 23:01:28 UTC) #5
Mohamed Mansour (USE mhm)
Okay, if you check it now, I received the bounds of the browser window, and ...
12 years ago (2008-12-17 23:34:39 UTC) #6
Evan Stade
http://codereview.chromium.org/14812/diff/6/205 File chrome/browser/browser.cc (right): http://codereview.chromium.org/14812/diff/6/205#newcode1336 Line 1336: browser_bounds.set_width(window()->GetNormalBounds().width()); um, what? delete lines 1336-1337
12 years ago (2008-12-17 23:55:26 UTC) #7
Evan Stade
sorry, ignore my comment. I am confused by this code though, can you add a ...
12 years ago (2008-12-17 23:56:33 UTC) #8
Mohamed Mansour (USE mhm)
I added more comments.
12 years ago (2008-12-18 00:12:33 UTC) #9
Peter Kasting
http://codereview.chromium.org/14812/diff/206/207 File chrome/browser/browser.cc (right): http://codereview.chromium.org/14812/diff/206/207#newcode1334 Line 1334: // The new popup window should have the ...
12 years ago (2008-12-18 00:22:01 UTC) #10
Elliot Glaysher
On 2008/12/18 00:12:33, Mohamed Mansour wrote: > I added more comments. I agree with Peter ...
12 years ago (2008-12-18 00:28:46 UTC) #11
Mohamed Mansour (USE mhm)
12 years ago (2008-12-18 00:41:09 UTC) #12
> Fix Peter's naming issue and I'll go ahead and commit it.
done.

Powered by Google App Engine
This is Rietveld 408576698