Note that I've updated the patch to refer to tip-of-tree.

========================================================================
http://mondrian.corp.google.com/file/9879962///depot/googleclient/gears/opensource/gears/ui/common/html_dialog.css?a=1
File //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.css 
(snapshot 1)
------------------------------------
Line 112: flow and the dialog displays correctly)
Should this comment really be identical to that for Android?!
------------------------------------
Line 143: ~)
The ID buttons-wince only appears in blocks guarded by IEMOBILE, so this switch
is redundant. (I agree that buttons-wince is a slightly confusing name).
========================================================================
http://mondrian.corp.google.com/file/9879962///depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js?a=1
File //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js 
(snapshot 1)
------------------------------------
Line 282: }
We've recently made changes so that this test is moved to the call site for PIE.
I think we should do the same for Opera.
========================================================================
http://mondrian.corp.google.com/file/9879962///depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4?a=1
File 
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4
 (snapshot 1)
------------------------------------
Line 37: ~~)
Does adding an empty title on other platforms cause problems? Why is it required
for Opera?
------------------------------------
Line 261: m4_ifelse(PRODUCT_BROWSER,~IE~,m4_dnl
This isn't needed now that we switch on IEMOBILE.
------------------------------------
Line 316: setButtonLabel("string-never-allow", "never-allow-button");
There is no element with ID never-allow-button.
------------------------------------
Line 434: m4_ifelse(PRODUCT_OS,~wince~,m4_dnl
We only use m4 switches where we have to - eg in CSS. In JavaScript, you can use
browser.opera and browser.wince - see base.js.
========================================================================
http://mondrian.corp.google.com/file/9879962///depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4?a=1
File 
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4 
(snapshot 1)
------------------------------------
Line 305: if (!browser.android && !browser.opera) {
Why do you test browser.android inside a WinCE only block? In any case, you can
use JavaScript for this test.
========================================================================
http://mondrian.corp.google.com/file/9879962///depot/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4?a=1
File 
//depot/googleclient/gears/opensource/gears/ui/common/shortcuts_dialog.html_m4 
(snapshot 1)
------------------------------------
Line 257: ~)
Again, you can do this test using JavaScript only.
------------------------------------
Line 303: ~)
Same again.
========================================================================

-- 
To respond, reply to this email or visit http://mondrian.corp.google.com/9879962

Reply via email to