Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9879962
or point your web browser to
http://mondrian/9879962
to review the following code:
Change 9879962 by stevebl...@steveblock-gears2 on 2009/01/27 19:10:40 *pending*
Modifies HTML dialogs for Opera. Mailed on behalf of Opera.
R=andreip
[email protected],[email protected],[email protected]
DELTA=133 (123 added, 0 deleted, 10 changed)
OCL=9879962
Affected files ...
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.css#6 edit
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#13 edit
...
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#7
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#6
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4#9
edit
133 delta lines: 123 added, 0 deleted, 10 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 9879962 by stevebl...@steveblock-gears2 on 2009/01/27 19:10:40 *pending*
Modifies HTML dialogs for Opera. Mailed on behalf of Opera.
Affected files ...
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.css#6 edit
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#13 edit
...
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#7
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#6
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4#9
edit
==== //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.css#6 -
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/ui/common/html_dialog.css
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/common/html_dialog.css
2009-01-27 19:25:43.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/html_dialog.css
2009-01-27 19:10:45.000000000 +0000
@@ -30,7 +30,21 @@
padding:0;
margin:0;
height:100%;
+
+m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
+~
+m4_ifelse(PRODUCT_BROWSER,~OPERA~,m4_dnl
+~
+ /*
+ On Opera Mobile we use overflow:auto to allow content to resize automatically
+ */
+ overflow:auto;
+~,~
overflow:hidden;
+~)
+~,~
+ overflow:hidden;
+~)
cursor:default;
/*
@@ -85,7 +99,25 @@
flow and the dialog displays correctly)
*/
~,~
+m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
+~
+m4_ifelse(PRODUCT_BROWSER,~OPERA~,m4_dnl
+~
+ /*
+ On Opera Mobile we do not use position:absolute due to some
+ rendering limitations (absolute positioning works, but
+ the automatic resizing of dialog is confused by the
+ variable content and the display is wrong, while if we
+ remove the absolute positioning, #foot is back in the
+ flow and the dialog displays correctly)
+ */
+~,~
position:absolute;
+~)
+~,~
+ position:absolute;
+~)
+
~)
background:white;
bottom:0;
@@ -97,11 +129,18 @@
}
#buttons-wince {
+m4_ifelse(PRODUCT_BROWSER,~OPERA~,m4_dnl
+~
+ /*
+ On Opera Mobile we do not hide buttons
+ */
+~,~
/*
On Windows Mobile, we hide the div containing the buttons by default, as
they are only used on touchscreen devices.
*/
display:none;
+~)
}
.accesskey {
==== //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#13 -
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
2009-01-27 19:25:43.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/html_dialog.js
2009-01-27 19:10:48.000000000 +0000
@@ -140,6 +140,8 @@
argsString = getFirefoxArguments(window.arguments[0]);
} else if (browser.safari) {
argsString = window.gears_dialogArguments;
+ } else if (browser.opera) {
+ argsString = window.dialogArguments;
} else if (browser.android) {
argsString = "" + window.bridge.getDialogArguments();
} else if (browser.chrome) {
@@ -179,6 +181,9 @@
window.close();
} else if (browser.safari) {
window.gears_dialog.setResults(resultString);
+ } else if (browser.opera) {
+ window.returnValue = resultString;
+ window.close();
} else if (browser.android) {
window.bridge.closeDialog(resultString);
} else if (browser.chrome) {
@@ -272,6 +277,10 @@
* e.g. windows mobile devices)
*/
function wrapDomain(str) {
+ if (!browser.ie_mobile && !browser.opera) {
+ 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.
====
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#7
-
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
2009-01-27 19:25:44.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4
2009-01-27 19:14:05.000000000 +0000
@@ -30,6 +30,11 @@
<html>
<head>
+ <meta name="viewport" content="width=device-width,user-scalable=no">
+m4_ifelse(PRODUCT_BROWSER,~OPERA~,m4_dnl
+~<title></title>~
+,
+~~)
<style type="text/css">
m4_ifelse(PRODUCT_OS,~android~,~~,~m4_dnl
@@ -252,8 +257,11 @@
</div>
m4_ifelse(PRODUCT_BROWSER,~IEMOBILE~,m4_dnl
-~<object style="display:none;"
classid="clsid:134AB400-1A81-4fc8-85DD-29CD51E9D6DE" id="pie_dialog">
-</object>~)
+~
+m4_ifelse(PRODUCT_BROWSER,~IE~,m4_dnl
+~<object style="display:none;"
classid="clsid:134AB400-1A81-4fc8-85DD-29CD51E9D6DE" id="pie_dialog">~m4_dnl
+~</object>~)
+~)
</body>
<!--
@@ -304,6 +312,9 @@
CustomButton.initializeAll();
}
+ if (browser.opera) {
+ setButtonLabel("string-never-allow", "never-allow-button");
+ }
if (browser.ie_mobile) {
var allowText = dom.getElementById("string-allow");
if (allowText) {
@@ -370,7 +381,7 @@
if (!customName) {
elem = dom.getElementById("origin-only");
elem.style.display = "block";
- if (browser.ie_mobile) {
+ if (browser.ie_mobile || browser.opera) {
elem.innerHTML = wrapDomain(origin);
} else {
setTextContent(elem, origin);
@@ -385,7 +396,7 @@
} else {
elem = dom.getElementById("origin");
elem.style.display = "block";
- if (browser.ie_mobile) {
+ if (browser.ie_mobile || browser.opera) {
elem.innerHTML = wrapDomain(origin);
} else {
setTextContent(elem, origin);
@@ -418,7 +429,17 @@
dom.getElementById("checkbox-row").style.display = 'block';
updateAllowButtonEnabledState();
}
+
+ // Do not resize on Opera Mobile, let the browser resize automatically
+m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
+~
+ if (!browser.opera)
+ resizeDialogToFitContent();
+~,
+~
resizeDialogToFitContent();
+~)
+
}
====
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#6
-
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
2009-01-27 19:25:44.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4
2009-01-27 19:16:15.000000000 +0000
@@ -30,6 +30,11 @@
<html>
<head>
+ <meta name="viewport" content="width=device-width,user-scalable=no">
+m4_ifelse(PRODUCT_BROWSER,~OPERA~,m4_dnl
+~<title></title>~
+,
+~~)
<style type="text/css">
m4_ifelse(PRODUCT_OS,~android~,~~,~m4_dnl
@@ -128,7 +133,14 @@
}
m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
-~~,~
+~
+m4_ifelse(PRODUCT_BROWSER,~OPERA~,m4_dnl
+~
+ div.radio-buttons {
+ width: 120px;
+ }
+~,~~)
+~,~
div.radio-buttons {
m4_ifelse(PRODUCT_OS,~android~,m4_dnl
~
@@ -233,9 +245,16 @@
</body>
-m4_ifelse(PRODUCT_BROWSER,~IEMOBILE~,m4_dnl
+<!--
+Note that this should be a switch on OS_WINCE and BROWSER_IE, but we don't have
+browser flags available here.
+-->
+m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
+~
+m4_ifelse(PRODUCT_BROWSER,~IE~,m4_dnl
~<object style="display:none;"
classid="clsid:134AB400-1A81-4fc8-85DD-29CD51E9D6DE" id="pie_dialog">~m4_dnl
~</object>~)
+~)
<script>
m4_include(../third_party/jsonjs/json_noeval.js)
@@ -279,9 +298,21 @@
if (!browser.ie_mobile) {
CustomButton.initializeAll();
+
+ // Do not initialize custom layout on Opera Mobile, let the browser layout
automatically
+m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
+~
+ if (!browser.android && !browser.opera) {
+ initCustomLayout(layoutSettings);
+ }
+~,
+~
if (!browser.android) {
initCustomLayout(layoutSettings);
}
+~)
+
+
} else {
var applyText = dom.getElementById("string-apply");
if (applyText) {
====
//depot/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4#9
-
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
2009-01-27 19:25:44.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4
2009-01-27 19:18:28.000000000 +0000
@@ -30,6 +30,11 @@
<html>
<head>
+ <meta name="viewport" content="width=device-width,user-scalable=no">
+m4_ifelse(PRODUCT_BROWSER,~OPERA~,m4_dnl
+~<title></title>~
+,
+~~)
<style type="text/css">
m4_ifelse(PRODUCT_OS,~android~,~~,~m4_dnl
@@ -217,8 +222,11 @@
</div>
m4_ifelse(PRODUCT_BROWSER,~IEMOBILE~,m4_dnl
-~<object style="display:none;"
classid="clsid:134AB400-1A81-4fc8-85DD-29CD51E9D6DE" id="pie_dialog">
-</object>~)
+~
+m4_ifelse(PRODUCT_BROWSER,~IE~,m4_dnl
+~<object style="display:none;"
classid="clsid:134AB400-1A81-4fc8-85DD-29CD51E9D6DE" id="pie_dialog">~m4_dnl
+~</object>~)
+~)
</body>
<!--
@@ -239,8 +247,14 @@
var debug = false;
var dialogMinHeight = 200;
- // We currently only support custom locations for desktop windows.
- var locationsEnabled = browser.windows;
+ // We currently only support custom locations for desktop windows. Opera
doesnt have a variable to indicate mobile,
+ // so use wince flag and browser.opera to distinguish
+m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
+~
+ var locationsEnabled = browser.windows && !browser.ie_mobile &&
!browser.opera;
+~,~
+ var locationsEnabled = browser.windows && !browser.ie_mobile;
+~)
var args;
@@ -277,7 +291,16 @@
initDialog();
initLayout();
initShortcuts(args);
+
+ // Do not resize on Opera Mobile, let the browser resize automatically
+m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
+~
+ if (!browser.opera)
+ resizeDialogToFitContent(dialogMinHeight);
+~,
+~
resizeDialogToFitContent(dialogMinHeight);
+~)
function initLayout() {
if (locationsEnabled) {
