Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
10832148:
Send e-mail to the verizon portal. (Closed)
|
Created:
8 years, 4 months ago by rkc Modified:
8 years, 4 months ago CC:
chromium-reviews, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, oshima+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionSend e-mail to the verizon portal.
Add the e-mail field to the data we post to the Verizon portal. The portal on the other end will pick it up and use it to pre-populate the field on the payment page. This data is also mandatory for 4g devices.
R=zelidrag@chromium.org,nkostylev@chromium.org
BUG=134596
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150139
Patch Set 1 #
Total comments: 9
Patch Set 2 : . #Patch Set 3 : #Messages
Total messages: 13 (0 generated)
https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... File chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc (right): https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:326: post_data.append(kEmailParam).append( no encoding here?
https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... File chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc (right): https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:326: post_data.append(kEmailParam).append( On 2012/08/03 22:29:55, zel wrote: > no encoding here? The post data is escaped in JS: http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/resources/c...
https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... File chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc (right): https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:325: if (manager) { I don't think you need such check. If UserManager instance is not created for some reason, we should crash. https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:327: manager->GetLoggedInUser().display_email()); Add check if (manager->IsUserLoggedIn())
https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... File chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc (right): https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:327: manager->GetLoggedInUser().display_email()); On 2012/08/03 22:59:01, Nikita Kostylev wrote: > Add check > if (manager->IsUserLoggedIn()) Otherwise it will return NULL when not in user session (at login screen).
https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... File chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc (right): https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:325: if (manager) { On 2012/08/03 22:59:01, Nikita Kostylev wrote: > I don't think you need such check. If UserManager instance is not created for > some reason, we should crash. Done. https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:327: manager->GetLoggedInUser().display_email()); On 2012/08/03 22:59:01, Nikita Kostylev wrote: > Add check > if (manager->IsUserLoggedIn()) Thanks for catching this; I keep forgetting that we can now activate from the login screen as well. Done.
https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... File chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc (right): https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:326: post_data.append(kEmailParam).append( On 2012/08/03 22:47:46, g_rkc wrote: > On 2012/08/03 22:29:55, zel wrote: > > no encoding here? > > The post data is escaped in JS: > http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/resources/c... no, that's not the same thing. it's escaped there to make sure the payload does not interfere with other params, but you need to escape stuff here to make sure your initial post payload is legit.
https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... File chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc (right): https://chromiumcodereview.appspot.com/10832148/diff/1/chrome/browser/ui/webu... chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc:326: post_data.append(kEmailParam).append( On 2012/08/03 23:09:01, zel wrote: > On 2012/08/03 22:47:46, g_rkc wrote: > > On 2012/08/03 22:29:55, zel wrote: > > > no encoding here? > > > > The post data is escaped in JS: > > > http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/resources/c... > > no, that's not the same thing. it's escaped there to make sure the payload does > not interfere with other params, but you need to escape stuff here to make sure > your initial post payload is legit. Done.
lgtm
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/10832148/9001
Change committed as 150139 |
