Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 10035046
or point your web browser to
http://mondrian/10035046
to review the following code:
Change 10035046 by stevebl...@steveblock-gears2 on 2009/02/06 13:19:06 *pending*
Breaks long domain names in permissions and settings dialogs.
R=andreip
[email protected]
DELTA=35 (10 added, 12 deleted, 13 changed)
OCL=10035046
Affected files ...
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.css#7 edit
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#14 edit
...
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#8
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#7
edit
35 delta lines: 10 added, 12 deleted, 13 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 10035046 by stevebl...@steveblock-gears2 on 2009/02/06 13:19:06 *pending*
Breaks long domain names in permissions and settings dialogs.
OCL=10035046
Affected files ...
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.css#7 edit
... //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#14 edit
...
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#8
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#7
edit
==== //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.css#7 -
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-02-06 14:41:48.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/html_dialog.css
2009-02-06 13:02:16.000000000 +0000
@@ -131,3 +131,8 @@
/* IE CSS extension */
accelerator: true;
}
+
+wbr:after {
+ /* Opera does not support <wbr>, so we use ​ instead. */
+ content : "\00200B"
+}
==== //depot/googleclient/gears/opensource/gears/ui/common/html_dialog.js#14 -
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-02-05 17:45:38.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/html_dialog.js
2009-02-06 13:39:41.000000000 +0000
@@ -273,16 +273,21 @@
}
/**
- * Returns a wrapped domain (useful for small screens dialogs,
- * e.g. windows mobile devices)
- */
-function wrapDomain(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.
- var dotImage = "<img height='2px' width='2px' " +
- "style='background-color: black; margin: 0px 1px;'>";
- return str.replace(/[.]/g, dotImage);
+ * Modifies a string to allow line breaks after each occurence of the specified
+ * string. escapedSearchString is the string escaped for use with regex, if
+ * required. eg '.' -> '\\.'
+ */
+function breakString(str, searchString, escapedSearchString) {
+ if (!escapedSearchString) {
+ escapedSearchString = searchString;
+ }
+ // Add an optional break after each occurence of searchString.
+ var optionalLineBreak = '<wbr>';
+ if (browser.ie_mobile) {
+ optionalLineBreak = '<img width="0px" height="0px">';
+ }
+ var regex = new RegExp(escapedSearchString, 'g');
+ return str.replace(regex, searchString + optionalLineBreak);
}
/**
====
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4#8
-
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-02-06 14:41:48.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/permissions_dialog.html_m4
2009-02-06 13:02:46.000000000 +0000
@@ -372,12 +372,8 @@
if (!customName) {
elem = dom.getElementById("origin-only");
elem.style.display = "block";
- if (browser.ie_mobile || (browser.wince && browser.opera)) {
- elem.innerHTML = wrapDomain(origin);
- } else {
- setTextContent(elem, origin);
- }
-
+ elem.innerHTML = breakString(origin, '.', '\\.');
+
// 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.
@@ -387,11 +383,7 @@
} else {
elem = dom.getElementById("origin");
elem.style.display = "block";
- if (browser.ie_mobile || (browser.wince && browser.opera)) {
- elem.innerHTML = wrapDomain(origin);
- } else {
- setTextContent(elem, origin);
- }
+ elem.innerHTML = breakString(origin, '.', '\\.');
}
if (customIcon) {
====
//depot/googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4#7
-
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-02-05 17:45:38.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/common/settings_dialog.html_m4
2009-02-06 13:03:38.000000000 +0000
@@ -386,11 +386,7 @@
}
content += "><tbody>";
content += "<tr><td class=\"origin-name\">";
- if (browser.ie_mobile || (browser.wince && browser.opera)) {
- content += wrapDomain(originData.name);
- } else {
- content += originData.name;
- }
+ content += breakString(originData.name, '.', '\\.');
content += "</td><td><a href='#' onclick='handleRemoveClick(";
content += index;
content += ");'>";
