[uber page] Remove obsolete subpage logic.

[uber page] Remove obsolete subpage logic.

BUG=116107
TEST=Verify that options page opens, no js errors, functional.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=125074

[csilv, 2012-03-06 00:14:16]

+jhawkins, dbeam for review.

[James Hawkins, 2012-03-06 00:15:42]

lgtm

[Dan Beam, 2012-03-06 00:31:41]

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
File chrome/browser/resources/options2/options_page.js (left):

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
chrome/browser/resources/options2/options_page.js:173:
OptionsPage.updateAriaHiddenForPages_ = function() {
don't we want to do this for overlays?

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
File chrome/browser/resources/options2/options_page.js (right):

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
chrome/browser/resources/options2/options_page.js:76: * Shows a registered page.
nit: or dialog/overlay.

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
chrome/browser/resources/options2/options_page.js:719: var container =
this.isOverlay ? this.pageDiv.parentNode : null;
this looks a little strange to me, but I guess it's cool. I think I'd expect to
see:

  if (!this.isOverlay)
    return;

  var contain = this.pageDiv.parentNode;

but there's no functional difference...

[csilv, 2012-03-06 01:03:03]

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
File chrome/browser/resources/options2/options_page.js (left):

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
chrome/browser/resources/options2/options_page.js:173:
OptionsPage.updateAriaHiddenForPages_ = function() {
On 2012/03/06 00:31:41, Dan Beam wrote:
> don't we want to do this for overlays?

Perhaps, but that should probably be in overlay.js rather than here.  Note that
in this implementation (and in the old settings page), this method is invoked
*only* when a sub-page is shown or hidden.  So currently, it is simply dead
code, thus the removal.

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
File chrome/browser/resources/options2/options_page.js (right):

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
chrome/browser/resources/options2/options_page.js:76: * Shows a registered page.
On 2012/03/06 00:31:41, Dan Beam wrote:
> nit: or dialog/overlay.

Done.

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
chrome/browser/resources/options2/options_page.js:719: var container =
this.isOverlay ? this.pageDiv.parentNode : null;
On 2012/03/06 00:31:41, Dan Beam wrote:
> this looks a little strange to me, but I guess it's cool. I think I'd expect
to
> see:
> 
>   if (!this.isOverlay)
>     return;
> 
>   var contain = this.pageDiv.parentNode;
> 
> but there's no functional difference...

Done.

[Dan Beam, 2012-03-06 01:57:37]

lgtm

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
File chrome/browser/resources/options2/options_page.js (left):

http://codereview.chromium.org/9554007/diff/6/chrome/browser/resources/option...
chrome/browser/resources/options2/options_page.js:173:
OptionsPage.updateAriaHiddenForPages_ = function() {
On 2012/03/06 01:03:04, csilv wrote:
> On 2012/03/06 00:31:41, Dan Beam wrote:
> > don't we want to do this for overlays?
> 
> Perhaps, but that should probably be in overlay.js rather than here.  Note
that
> in this implementation (and in the old settings page), this method is invoked
> *only* when a sub-page is shown or hidden.  So currently, it is simply dead
> code, thus the removal.

I see what you mean, but it seems reasonable to do it partially here, though, as
overlay.js probably shouldn't know about:

  a) the relation of each overlay to eachother
  b) about options pages

right? Maybe we could just add handlers in OptionPage's (and derived)
didShowPage() (as well as pass a boolean to say whether it'll be the foreground
page as well) or didClosePage() and each overlay could affect itself or it's
pageDiv.

Either way, Evan told me this didn't work for overlays/dialogs previously, so
nothing is being lost here, so we'll just do this in a different CL, :).