A cockroach-sized code review (wangxianzhu localrev 1301)

4 views
Skip to first unread message

phni...@gmail.com

unread,
Jan 17, 2010, 9:27:06 PM1/17/10
to jame...@gmail.com, google-gadgets...@googlegroups.com
Hello james.su,

I'd like you to do a code review. Please review the following patch:

----------------------------------------------------------------------
r1301: (no author) | 2010-01-18 10:25:12 +0800

About iGoogle gadget:
1. Added "Add an iGoogle Gadget" in main menu
2. Allow pure iGoogle gadget URLs
3. Modified the hint message for adding iGoogle Gadgets.
----------------------------------------------------------------------

=== extensions/google_gadget_manager/google_gadget_manager.cc
==================================================================
--- extensions/google_gadget_manager/google_gadget_manager.cc (revision 1300)
+++ extensions/google_gadget_manager/google_gadget_manager.cc (revision 1301)
@@ -405,12 +405,18 @@
int instance_id) {
std::string options_name = GetGadgetInstanceOptionsName(instance_id);
OptionsInterface *instance_options = CreateOptions(options_name.c_str());
+
Variant org_gadget_id =
instance_options->GetInternalValue(kInstanceGadgetIdOption);
- if (org_gadget_id == Variant(gadget_id)) {
- // The existing options can be reused.
- delete instance_options;
- return true;
+
+ // Options won't be reused for blank igoogle gadgets and rss gadgets.
+ if (strcmp(gadget_id, kIGoogleGadgetName) != 0 &&
+ strcmp(gadget_id, kRSSGadgetName) != 0) {
+ if (org_gadget_id == Variant(gadget_id)) {
+ // The existing options can be reused.
+ delete instance_options;
+ return true;
+ }
}

if (org_gadget_id.type() != Variant::TYPE_VOID) {
@@ -513,7 +519,7 @@
}

int GoogleGadgetManager::NewGadgetInstanceFromFile(const char *file) {
- return GadgetIdIsFileLocation(file) ? NewGadgetInstance(file) : -1;
+ return NewGadgetInstance(file);
}

bool GoogleGadgetManager::RemoveGadgetInstanceInternal(int instance_id,
@@ -971,7 +977,7 @@
return file_manager_->GetFullPath(
GetDownloadedGadgetLocation(gadget_id).c_str());
}
-
+
result.clear();
StringMap::const_iterator module_id_it = info->attributes.find(kModuleIDAttrib);
if (module_id_it != info->attributes.end())
=== extensions/google_gadget_manager/google_gadget_manager.h
==================================================================
--- extensions/google_gadget_manager/google_gadget_manager.h (revision 1300)
+++ extensions/google_gadget_manager/google_gadget_manager.h (revision 1301)
@@ -145,11 +145,9 @@
* The following are constants used for feed and iGoogle gadgets.
* These declarations must match those in the corresponding gadget .js files.
*/
-const char kRSSGadgetName[] = "rss";
const char kRSSURLOption[] = "rss_url";
const char kRSSModuleID[] = "25";
const char kIGoogleModuleID[] = "32";
-const char kIGoogleGadgetName[] = "igoogle";
const char kIGoogleURLOption[] = "download_url";
/** The following two are optional. */
const char kIGoogleModuleURLOption[] = "module_url_prefix";
=== hosts/qt/qt_host_internal.h
==================================================================
--- hosts/qt/qt_host_internal.h (revision 1300)
+++ hosts/qt/qt_host_internal.h (revision 1301)
@@ -65,6 +65,8 @@
qApp->setQuitOnLastWindowClosed(false);
menu_.addAction(QString::fromUtf8(GM_("MENU_ITEM_ADD_GADGETS")),
this, SLOT(OnAddGadget()));
+ menu_.addAction(QString::fromUtf8(GM_("MENU_ITEM_ADD_IGOOGLE_GADGET")),
+ this, SLOT(OnAddIGoogleGadget()));
menu_.addAction(QString::fromUtf8(GM_("MENU_ITEM_SHOW_ALL")),
this, SLOT(OnShowAll()));
menu_.addAction(QString::fromUtf8(GM_("MENU_ITEM_HIDE_ALL")),
@@ -380,6 +382,10 @@
GetGadgetManager()->ShowGadgetBrowserDialog(&gadget_browser_host_);
}

+ void OnAddIGoogleGadget() {
+ GetGadgetManager()->NewGadgetInstanceFromFile(kIGoogleGadgetName);
+ }
+
void OnGadgetMenuItem() {
QAction *s = static_cast<QAction*>(sender());
if (s && gadget_menu_map_.contains(s)) {
=== hosts/gtk/sidebar_gtk_host.cc
==================================================================
--- hosts/gtk/sidebar_gtk_host.cc (revision 1300)
+++ hosts/gtk/sidebar_gtk_host.cc (revision 1301)
@@ -365,6 +365,9 @@
menu->AddItem(GM_("MENU_ITEM_ADD_GADGETS"), 0,
MenuInterface::MENU_ITEM_ICON_ADD,
NewSlot(this, &Impl::AddGadgetMenuHandler), priority);
+ menu->AddItem(GM_("MENU_ITEM_ADD_IGOOGLE_GADGET"), 0,
+ MenuInterface::MENU_ITEM_ICON_ADD,
+ NewSlot(this, &Impl::AddIGoogleGadgetMenuHandler), priority);
menu->AddItem(NULL, 0, 0, NULL, priority);
menu->AddItem(GM_("MENU_ITEM_SIDEBAR"),
(closed_ ? 0 : MenuInterface::MENU_ITEM_FLAG_CHECKED), 0,
@@ -1624,6 +1627,11 @@
gadget_manager_->ShowGadgetBrowserDialog(&gadget_browser_host_);
}

+ void AddIGoogleGadgetMenuHandler(const char *str) {
+ GGL_UNUSED(str);
+ gadget_manager_->NewGadgetInstanceFromFile(kIGoogleGadgetName);
+ }
+
void ShowAllMenuHandler(const char *str) {
GGL_UNUSED(str);
ShowOrHideAll(true);
=== hosts/gtk/simple_gtk_host.cc
==================================================================
--- hosts/gtk/simple_gtk_host.cc (revision 1300)
+++ hosts/gtk/simple_gtk_host.cc (revision 1301)
@@ -161,6 +161,10 @@
MenuInterface::MENU_ITEM_ICON_ADD,
NewSlot(this, &Impl::AddGadgetMenuCallback),
priority);
+ menu_builder.AddItem(GM_("MENU_ITEM_ADD_IGOOGLE_GADGET"), 0,
+ MenuInterface::MENU_ITEM_ICON_ADD,
+ NewSlot(this, &Impl::AddIGoogleGadgetMenuCallback),
+ priority);
menu_builder.AddItem(GM_("MENU_ITEM_SHOW_ALL"), 0, 0,
NewSlot(this, &Impl::ShowAllMenuCallback),
priority);
@@ -513,6 +517,10 @@
gadget_manager_->ShowGadgetBrowserDialog(&gadget_browser_host_);
}

+ void AddIGoogleGadgetMenuCallback(const char *) {
+ gadget_manager_->NewGadgetInstanceFromFile(kIGoogleGadgetName);
+ }
+
void OnCloseHandler(DecoratedViewHost *decorated) {
ViewInterface *child = decorated->GetView();
Gadget *gadget = child ? child->GetGadget() : NULL;
=== ggadget/gadget_manager_interface.h
==================================================================
--- ggadget/gadget_manager_interface.h (revision 1300)
+++ ggadget/gadget_manager_interface.h (revision 1301)
@@ -27,6 +27,10 @@
class Connection;
template <typename R, typename P1> class Slot1;

+// Names of some built-in gadgets.
+const char kRSSGadgetName[] = "rss";
+const char kIGoogleGadgetName[] = "igoogle";
+
/**
* @ingroup Interfaces
* @ingroup Gadget
@@ -45,11 +49,11 @@
virtual void Init() = 0;

/**
- * Creates an new instance of a gadget specified by the file path. Used to
- * open a gadget located in local file system.
- * @param file location of a gadget file. The location can be a full path of
- * a gadget file, or a location that can be recognized by the global
- * file manager.
+ * Creates an new instance of a gadget specified by the file path or the
+ * name of a built-in gadget.
+ * @param file location of a gadget file or name of a built-in gadget.
+ * The location can be a full path of a gadget file, or a location
+ * that can be recognized by the global file manager.
* @return the gadget instance id (>=0) of the new instance, or -1 on error.
*/
virtual int NewGadgetInstanceFromFile(const char *file) = 0;
=== resources/fr/strings.xml
==================================================================
--- resources/fr/strings.xml (revision 1300)
+++ resources/fr/strings.xml (revision 1301)
@@ -82,6 +82,7 @@
even in the middle of a word. -->
<MENU_ITEM_ABOUT>A Propos...</MENU_ITEM_ABOUT>
<MENU_ITEM_ADD_GADGETS>Ajouter des Gadgets...</MENU_ITEM_ADD_GADGETS>
+<MENU_ITEM_ADD_IGOOGLE_GADGET>Ajouter un Gadget iGoogle</MENU_ITEM_ADD_IGOOGLE_GADGET>
<MENU_ITEM_ALWAYS_ON_TOP>Toujours au premier plan</MENU_ITEM_ALWAYS_ON_TOP>
<MENU_ITEM_AUTO_FIT>Redimensionner automatiquement</MENU_ITEM_AUTO_FIT>
<MENU_ITEM_AUTO_HIDE>Masquer automatiquement</MENU_ITEM_AUTO_HIDE>
=== resources/zh-CN/strings.xml
==================================================================
--- resources/zh-CN/strings.xml (revision 1300)
+++ resources/zh-CN/strings.xml (revision 1301)
@@ -82,6 +82,7 @@
even in the middle of a word. -->
<MENU_ITEM_ABOUT>关于(&amp;A)...</MENU_ITEM_ABOUT>
<MENU_ITEM_ADD_GADGETS>添加小工具(&amp;G)...</MENU_ITEM_ADD_GADGETS>
+<MENU_ITEM_ADD_IGOOGLE_GADGET>添加 iGoogle 小工具(&amp;I)</MENU_ITEM_ADD_IGOOGLE_GADGET>
<MENU_ITEM_ALWAYS_ON_TOP>总在最前(&amp;T)</MENU_ITEM_ALWAYS_ON_TOP>
<MENU_ITEM_AUTO_FIT>自适应(&amp;F)</MENU_ITEM_AUTO_FIT>
<MENU_ITEM_AUTO_HIDE>自动隐藏(&amp;U)</MENU_ITEM_AUTO_HIDE>
=== resources/en/strings.xml
==================================================================
--- resources/en/strings.xml (revision 1300)
+++ resources/en/strings.xml (revision 1301)
@@ -82,6 +82,7 @@
even in the middle of a word. -->
<MENU_ITEM_ABOUT>&amp;About...</MENU_ITEM_ABOUT>
<MENU_ITEM_ADD_GADGETS>Add &amp;Gadgets...</MENU_ITEM_ADD_GADGETS>
+<MENU_ITEM_ADD_IGOOGLE_GADGET>Add an &amp;iGoogle Gadget</MENU_ITEM_ADD_IGOOGLE_GADGET>
<MENU_ITEM_ALWAYS_ON_TOP>Always on &amp;Top</MENU_ITEM_ALWAYS_ON_TOP>
<MENU_ITEM_AUTO_FIT>Auto &amp;Fit</MENU_ITEM_AUTO_FIT>
<MENU_ITEM_AUTO_HIDE>A&amp;uto Hide</MENU_ITEM_AUTO_HIDE>
=== gadgets/igoogle/main.js
==================================================================
--- gadgets/igoogle/main.js (revision 1300)
+++ gadgets/igoogle/main.js (revision 1301)
@@ -403,21 +403,20 @@
/^\w+:\/\/\w+\.google\.\w+\/ig\/adde.*moduleurl=(.*.xml)/
];

- var gadget_url = null;
- var url = urlEntry.value;
- if (url != null) {
+ var gadget_url = urlEntry.value;
+ if (gadget_url) {
for (i in url_patterns) {
- var match_result = url.match(url_patterns[i]);
+ var match_result = gadget_url.match(url_patterns[i]);
if (match_result != null) {
gadget_url = match_result[1];
- if (!gadget_url.match(/^\w+:\/\//))
- gadget_url = "http://" + gadget_url;
break;
}
}
}

- if (gadget_url != null) {
+ if (gadget_url) {
+ if (!gadget_url.match(/^\w+:\/\//))
+ gadget_url = "http://" + gadget_url;
g_download_url = gadget_url;
options.putValue(kDownloadURLOption, g_download_url);
LoadGadget();
=== gadgets/igoogle/zh-CN/strings.xml
==================================================================
--- gadgets/igoogle/zh-CN/strings.xml (revision 1300)
+++ gadgets/igoogle/zh-CN/strings.xml (revision 1301)
@@ -26,8 +26,7 @@
<GADGET_NOTLOADED>正在加载选项...</GADGET_NOTLOADED>
<NO_OPTIONS>该小工具没有可配置的选项。</NO_OPTIONS>
<GADGET_DIRECTORY_URL>http://www.google.com/ig/directory</GADGET_DIRECTORY_URL>
-<GADGET_URL_HINT>在 iGoogle 小工具目录中选一个你喜爱的小工具,将小工具的URL粘贴或拖拽到下面的输入框中。然后点击“加载小工具”按钮。
-你可以通过点击小工具描述页面上的“共享该小工具”链接来获得小工具的URL。</GADGET_URL_HINT>
+<GADGET_URL_HINT>在 iGoogle 小工具目录中选一个你喜爱的小工具,将小工具的链接拖拽到下面的输入框中,或输入小工具的URL。然后点击“加载小工具”按钮。</GADGET_URL_HINT>
<LOAD_GADGET>加载小工具</LOAD_GADGET>
<INVALID_URL>无效的小工具URL.</INVALID_URL>
<GADGET_UNLOAD>卸载</GADGET_UNLOAD>
=== gadgets/igoogle/en/strings.xml
==================================================================
--- gadgets/igoogle/en/strings.xml (revision 1300)
+++ gadgets/igoogle/en/strings.xml (revision 1301)
@@ -26,8 +26,7 @@
<GADGET_NOTLOADED>Options loading...</GADGET_NOTLOADED>
<NO_OPTIONS>This gadget has no configurable option.</NO_OPTIONS>
<GADGET_DIRECTORY_URL>http://www.google.com/ig/directory</GADGET_DIRECTORY_URL>
-<GADGET_URL_HINT>Find a favorite iGoogle Gadget in Gadget directory and paste (or drag) the URL into the entry box below. Then click &quot;Load Gadget&quot; button.
-You can find the URL by clicking &quot;Share this gadget&quot; link in gadget's description page.</GADGET_URL_HINT>
+<GADGET_URL_HINT>Find a favorite iGoogle Gadget in Gadget directory and drag the link into the entry box below, or enter the gadget URL. Then click &quot;Load Gadget&quot; button.</GADGET_URL_HINT>
<LOAD_GADGET>Load Gadget</LOAD_GADGET>
<INVALID_URL>Invalid Gadget URL.</INVALID_URL>
<GADGET_UNLOAD>Unload</GADGET_UNLOAD>
=== gadgets/igoogle/fr/strings.xml
==================================================================
--- gadgets/igoogle/fr/strings.xml (revision 1300)
+++ gadgets/igoogle/fr/strings.xml (revision 1301)
@@ -26,7 +26,7 @@
<GADGET_NOTLOADED>Options en cours de chargement...</GADGET_NOTLOADED>
<NO_OPTIONS>Ce gadget n'a aucune option paramétrable.</NO_OPTIONS>
<GADGET_DIRECTORY_URL>http://www.google.com/ig/directory</GADGET_DIRECTORY_URL>
-<GADGET_URL_HINT>Trouvez un Gadget iGoogle favori dans le répertoire de Gadgets et collez (ou déplacez) l'URL dans la boîte de saisie ci-dessous. Ensuite, cliquez sur le bouton &quot;Charger un Gadget&quot;. Vous pouvez trouver l'URL en cliquant sur le lien &quot;Partager ce gadget&quot; dans la page de description du gadget.</GADGET_URL_HINT>
+<GADGET_URL_HINT>Trouvez un Gadget iGoogle favori dans le répertoire de Gadgets et collez le lien dans la boîte de saisie ci-dessous, ou entrer l'URL du gadget. Ensuite, cliquez sur le bouton &quot;Charger un Gadget&quot;.</GADGET_URL_HINT>
<LOAD_GADGET>Charger un Gadget</LOAD_GADGET>
<INVALID_URL>URL de Gadget Invalide.</INVALID_URL>
<GADGET_UNLOAD>Décharger</GADGET_UNLOAD>

This is a semiautomated message from "svkmail". Complaints or suggestions?
Mail edy...@gmail.com.

James Su

unread,
Jan 17, 2010, 10:57:23 PM1/17/10
to google-gadgets...@googlegroups.com
LGTM.

2010/1/18 <phni...@gmail.com>
Reply all
Reply to author
Forward
0 new messages