Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9501446
or point your web browser to
http://mondrian/9501446
to review the following code:
Change 9501446 by stevebl...@steveblock-gears2 on 2008/12/22 12:13:46 *pending*
Cleans up browser and OS detection for WinCE in dialogs.
This should simplify the logic when we modify the dialogs for Opera
Mobile.
R=andreip
[email protected]
DELTA=43 (13 added, 9 deleted, 21 changed)
OCL=9501446
Affected files ...
...
//depot/googleclient/gears/opensource/gears/ui/common/alert_dialog.html_m4#1
edit
... //depot/googleclient/gears/opensource/gears/ui/common/base.js#6 edit
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#12 edit
...
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#6
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#4
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4#6
edit
43 delta lines: 13 added, 9 deleted, 21 changed
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [email protected].
Change 9501446 by stevebl...@steveblock-gears2 on 2008/12/22 12:13:46 *pending*
Cleans up browser and OS detection for WinCE in dialogs.
This should simplify the logic when we modify the dialogs for Opera
Mobile.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/ui/common/alert_dialog.html_m4#1
edit
... //depot/googleclient/gears/opensource/gears/ui/common/base.js#6 edit
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#12 edit
...
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#6
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#4
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4#6
edit
====
//depot/googleclient/gears/opensource/gears/ui/common/alert_dialog.html_m4#1 -
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/ui/common/alert_dialog.html_m4
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/common/alert_dialog.html_m4
2008-12-22 13:32:25.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/alert_dialog.html_m4
2008-12-22 12:08:45.000000000 +0000
@@ -92,7 +92,7 @@
var debug = false;
if (debug) {
// Handy for debugging layout:
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
window.pie_dialog = new Object();
window.pie_dialog.IsSmartPhone = function() { return false; };
window.pie_dialog.SetCancelButton = function() {};
==== //depot/googleclient/gears/opensource/gears/ui/common/base.js#6 -
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/ui/common/base.js ====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/common/base.js 2008-10-08
09:50:53.000000000 +0100
+++ googleclient/gears/opensource/gears/ui/common/base.js 2008-12-22
12:04:08.000000000 +0000
@@ -127,19 +127,23 @@
var ua = navigator.userAgent;
browser.opera = typeof opera != "undefined";
browser.ie = !browser.opera && ua.indexOf("MSIE") > -1;
- browser.ie_mobile = (ua.indexOf("Windows CE") > -1) &&
- (ua.indexOf("MSIE") > -1);
browser.webkit = ua.indexOf("WebKit") > -1;
// WebKit also gives product == "gecko".
browser.mozilla = !browser.webkit && navigator.product == "Gecko";
browser.safari = ua.indexOf("Safari") > -1;
- browser.mac = navigator.platform.indexOf("Mac") > -1;
- browser.linux = navigator.platform.indexOf("Linux") > -1;
- browser.windows = navigator.platform.indexOf("Win") > -1;
browser.android = ua.indexOf("Android") > -1;
browser.chrome = window.chrome; // TODO(mpcomplete): use ua string
if (browser.android || browser.chrome) {
browser.safari = false;
}
+
+ browser.mac = navigator.platform.indexOf("Mac") > -1;
+ browser.linux = navigator.platform.indexOf("Linux") > -1;
+ browser.wince = navigator.platform.indexOf("WinCE") > -1 ||
+ navigator.platform.indexOf("Pocket PC") > -1 ||
+ navigator.platform.indexOf("Windows CE") > -1 ||
+ navigator.platform.indexOf("Windows Mobile") > -1;
+ browser.windows = !browser.wince && navigator.platform.indexOf("Win") > -1;
+
// TODO(aa): Add detection for more browsers, as necessary.
})();
==== //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#12 -
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/ui/common/html_dialog.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/common/html_dialog.js
2008-11-10 14:29:59.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/html_dialog.js
2008-12-22 12:09:38.000000000 +0000
@@ -27,7 +27,7 @@
* Initialize the base functionality of the dialog.
*/
function initDialog() {
- if (!browser.ie_mobile) {
+ if (!(browser.ie && browser.wince)) {
dom.addEvent(document, "keyup", handleKeyUp);
} else {
if (!window.pie_dialog.IsSmartPhone()) {
@@ -37,7 +37,7 @@
}
}
}
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
window.pie_dialog.SetScriptContext(window);
window.pie_dialog.ResizeDialog();
}
@@ -132,7 +132,7 @@
*/
function getArguments() {
var argsString;
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
argsString = window.pie_dialog.GetDialogArguments();
} else if (browser.ie) {
argsString = window.external.GetDialogArguments();
@@ -170,7 +170,7 @@
*/
function saveAndClose(resultObject) {
var resultString = JSON.stringify(resultObject);
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
window.pie_dialog.CloseDialog(resultString);
} else if (browser.ie) {
window.external.CloseDialog(resultString);
@@ -233,7 +233,7 @@
function disableButton(buttonElem) {
buttonElem.disabled = true;
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
window.pie_dialog.SetButtonEnabled(false);
}
@@ -253,7 +253,7 @@
function enableButton(buttonElem) {
buttonElem.disabled = false;
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
window.pie_dialog.SetButtonEnabled(true);
}
@@ -272,10 +272,6 @@
* e.g. windows mobile devices)
*/
function wrapDomain(str) {
- if (!browser.ie_mobile) {
- return str;
- }
-
// Replace occurences of '.' with an image representing a dot. This allows
the
// browser to wrap the URL at these points as if there were whitespace
// present.
@@ -289,7 +285,7 @@
*/
function resizeDialogToFitContent(opt_minDialogInnerHeight) {
// This does not work on PIE (no height measurement)
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
window.pie_dialog.ResizeDialog();
return;
}
====
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#6
-
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4
2008-11-10 14:29:59.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4
2008-12-22 12:11:18.000000000 +0000
@@ -282,7 +282,7 @@
var debug = false;
if (debug) {
// Handy for debugging layout:
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
window.pie_dialog = new Object();
window.pie_dialog.IsSmartPhone = function() { return false; };
window.pie_dialog.SetCancelButton = function() {};
@@ -308,11 +308,11 @@
text = text.replace(/t/, "<span class='accesskey'>t</span>");
trustTextElement.innerHTML = text;
- if (!browser.ie_mobile) {
+ if (!(browser.ie && browser.wince)) {
CustomButton.initializeAll();
}
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
var allowText = dom.getElementById("string-allow");
if (allowText) {
window.pie_dialog.SetButton(allowText.innerText,
"allowAccessPermanently();");
@@ -378,7 +378,7 @@
if (!customName) {
elem = dom.getElementById("origin-only");
elem.style.display = "block";
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
elem.innerHTML = wrapDomain(origin);
} else {
setTextContent(elem, origin);
@@ -387,13 +387,13 @@
// When all we have is the origin, we lay it out centered because that
// looks nicer. This is also what the original dialog did, which did not
// support the extra name, icon, or message.
- if (!browser.ie_mobile && !customIcon && !customMessage) {
+ if (!(browser.ie && browser.wince) && !customIcon && !customMessage) {
elem.setAttribute("align", "center");
}
} else {
elem = dom.getElementById("origin");
elem.style.display = "block";
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
elem.innerHTML = wrapDomain(origin);
} else {
setTextContent(elem, origin);
@@ -420,7 +420,7 @@
setTextContent(elem, customMessage);
}
- if (!browser.ie_mobile) {
+ if (!(browser.ie && browser.wince)) {
// Set up the checkbox to toggle the enabledness of the Allow button.
dom.getElementById("unlock").onclick = updateAllowButtonEnabledState;
dom.getElementById("checkbox-row").style.display = 'block';
====
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#4
-
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4
2008-11-10 14:29:59.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4
2008-12-22 12:12:03.000000000 +0000
@@ -264,7 +264,7 @@
var PERMISSION_ALLOWED = 1;
var PERMISSION_DENIED = 2;
- if (debug && browser.ie_mobile) {
+ if (debug && browser.ie && browser.wince) {
// Handy for debugging layout.
// Remember to remove the pie_dialog object tag above when debugging on
// WinMo 5, otherwise the assignement below will fail with "the object does
@@ -285,7 +285,7 @@
setButtonLabel("string-cancel", "cancel-button", "string-cancel-accesskey");
setButtonLabel("string-apply", "confirm-button", "string-apply-accesskey");
- if (!browser.ie_mobile) {
+ if (!(browser.ie && browser.wince)) {
CustomButton.initializeAll();
if (!browser.android) {
initCustomLayout(layoutSettings);
@@ -382,7 +382,11 @@
}
content += "><tbody>";
content += "<tr><td class=\"origin-name\">";
- content += wrapDomain(originData.name);
+ if (browser.ie && browser.wince) {
+ content += wrapDomain(originData.name);
+ } else {
+ content += originData.name;
+ }
content += "</td><td><a href='#' onclick='handleRemoveClick(";
content += index;
content += ");'>";
====
//depot/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4#6
-
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4
2008-11-10 14:29:59.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4
2008-12-22 12:10:37.000000000 +0000
@@ -246,13 +246,13 @@
var dialogMinHeight = 200;
// We currently only support custom locations for desktop windows.
- var locationsEnabled = browser.windows && !browser.ie_mobile;
+ var locationsEnabled = browser.windows;
var args;
if (debug) {
// Handy for debugging layout:
- if (browser.ie_mobile) {
+ if (browser.ie && browser.wince) {
window.pie_dialog = new Object();
window.pie_dialog.IsSmartPhone = function() { return false; };
window.pie_dialog.SetCancelButton = function() {};
@@ -295,7 +295,7 @@
// Gears).
if (isDefined(typeof args.style) && args.style == "simple") {
initSimpleStyle();
- } else if (browser.ie_mobile) {
+ } else if (browser.ie && browser.wince) {
initPieStyle();
} else {
initDefaultStyle();
