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

Unified Diff: chrome/browser/resources/uber/uber.js

Issue 300033009: Settings: load iframes anew when switching to a non-current one. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: dbeam comments Created 6 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/resources/uber/uber.js
diff --git a/chrome/browser/resources/uber/uber.js b/chrome/browser/resources/uber/uber.js
index 5888f6dbee74e4c231a90365fe880339e1407a96..1599882c03a905da1452abdd18868ba285cce55b 100644
--- a/chrome/browser/resources/uber/uber.js
+++ b/chrome/browser/resources/uber/uber.js
@@ -105,12 +105,11 @@ cr.define('uber', function() {
var params = resolvePageInfo();
assert(params.id === e.state.pageId);
- // If the page doesn't exist, create it. Otherwise, swap it in.
- var frame = $(params.id).querySelector('iframe');
- if (!frame)
+ // If the page isn't the current page, load it fresh. Even if the page is
+ // already loaded, it may have state not reflected in the URL, such as the
+ // history page's "Remove selected items" overlay. http://crbug.com/377386
+ if (getRequiredElement(params.id) !== getSelectedIframe())
showPage(params.id, HISTORY_STATE_OPTION.NONE, params.path);
- else
- selectPage(params.id);
// Either way, send the state down to it.
//
@@ -224,12 +223,11 @@ cr.define('uber', function() {
/**
* Changes the path past the page title (i.e. chrome://chrome/settings/(.*)).
- * @param {string} pageId Should match an id of one of the iframe containers.
* @param {Object} state The page's state object for the navigation.
* @param {string} path The new /path/ to be set after the page name.
* @param {number} historyOption The type of history modification to make.
*/
- function changePathTo(pageId, state, path, historyOption) {
+ function changePathTo(state, path, historyOption) {
assert(!path || path.substr(-1) != '/', 'invalid path given');
var histFunc;
@@ -240,6 +238,7 @@ cr.define('uber', function() {
assert(histFunc, 'invalid historyOption given ' + historyOption);
+ var pageId = getSelectedIframe().id;
var args = [{pageId: pageId, pageState: state},
'',
'/' + pageId + '/' + (path || '')];
@@ -261,7 +260,7 @@ cr.define('uber', function() {
// Only update the currently displayed path if this is the visible frame.
var container = getIframeFromOrigin(origin).parentNode;
if (container == getSelectedIframe())
- changePathTo(container.id, state, path, historyOption);
+ changePathTo(state, path, historyOption);
}
/**
@@ -341,23 +340,8 @@ cr.define('uber', function() {
frame.dataset.ready = false;
}
- // This must be called before selectPage so that the title change applies to
- // the new history entry.
- if (historyOption != HISTORY_STATE_OPTION.NONE)
- changePathTo(pageId, {}, path, historyOption);
-
- selectPage(pageId);
- }
-
- /**
- * Switches to a subpage. The subpage must already exist.
- * @param {string} pageId Should match an id of one of the iframe containers.
- */
- function selectPage(pageId) {
- var container = getRequiredElement(pageId);
- var lastSelected = document.querySelector('.iframe-container.selected');
-
// If the last selected container is already showing, ignore the rest.
+ var lastSelected = document.querySelector('.iframe-container.selected');
if (lastSelected === container)
return;
@@ -386,6 +370,9 @@ cr.define('uber', function() {
var selectedFrame = getSelectedIframe().querySelector('iframe');
uber.invokeMethodOnWindow(selectedFrame.contentWindow, 'frameSelected');
+ if (historyOption != HISTORY_STATE_OPTION.NONE)
+ changePathTo({}, path, historyOption);
+
if (container.dataset.title)
document.title = container.dataset.title;
$('favicon').href = 'chrome://theme/' + container.dataset.favicon;
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698