Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(9)

Issue 12225095: Interactive autofill: Adds footnote view to accept legal documents in the UI. (Closed)

Created:
6 years, 5 months ago by Dan Beam
Modified:
6 years, 3 months ago
CC:
chromium-reviews, Raman Kakilate, tfarina, benquan, dhollowa+watch_chromium.org, ahutter, browser-components-watch_chromium.org, dbeam+watch-autofill_chromium.org, Dane Wallinga, dyu1, estade+watch_chromium.org, Albert Bodenhamer, Ilya Sherman, groby-ooo-7-16
Visibility:
Public.

Description

Interactive autofill: Adds footnote view to accept legal documents in the UI. Adds IDS_AUTOFILL_DIALOG_LEGAL_DOC_LINKS_* and IDS_AUTOFILL_DIALOG_ACCEPT_CHANGES. Adds to AutofillDialogController{,Impl}: - std::vector<string16> FootnoteLinkParts(): label/link text parts (even are text, odd are links). - LegalDocumentLinkClicked(size_t index): callback when links are clicked (|index| is of the legal document). - string16 AcceptChangesText(): bottom line of footer, translated. Adds to AutofillDialogView{,s}: - UpdateFootnote(): called when the footnote should rebuild itself. Adds to AutofillDialogViews: - CreateFootnoteView(): creates an empty footnote. - RepopulateFootnoteLinks(): nukes current footnote links and re-creates them. BUG=168704

Patch Set 1 #

Patch Set 2 : . #

Total comments: 21

Patch Set 3 : . #

Patch Set 4 : <=3 #

Total comments: 2

Patch Set 5 : cruft #

Patch Set 6 : !android #

Patch Set 7 : closer? #

Total comments: 10

Patch Set 8 : . #

Patch Set 9 : . #

Patch Set 10 : . #

Total comments: 20

Patch Set 11 : . #

Patch Set 12 : estade@ / ahutter@ review #

Patch Set 13 : . #

Total comments: 1

Patch Set 14 : rebase #

Patch Set 15 : . #

Patch Set 16 : rebase #

Patch Set 17 : . #

Patch Set 18 : . #

Patch Set 19 : . #

Patch Set 20 : . #

Total comments: 4

Patch Set 21 : sky@ review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+558 lines, -131 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +22 lines, -2 lines 0 comments Download
M chrome/browser/autofill/wallet/wallet_items.h View 1 2 3 4 5 6 7 8 9 10 4 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/autofill/wallet/wallet_items.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/autofill/wallet/wallet_items_unittest.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/autofill/autofill_dialog_controller.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +25 lines, -12 lines 0 comments Download
M chrome/browser/ui/autofill/autofill_dialog_controller_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 10 chunks +144 lines, -67 lines 0 comments Download
M chrome/browser/ui/autofill/autofill_dialog_view.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/autofill/autofill_dialog_views.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 8 chunks +74 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/autofill/autofill_dialog_views.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 15 chunks +266 lines, -32 lines 0 comments Download
M ui/views/window/dialog_client_view.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +6 lines, -6 lines 0 comments Download

Messages

Total messages: 27 (0 generated)
Dan Beam
will fix description up later, too tired to think
6 years, 5 months ago (2013-02-08 07:17:34 UTC) #1
Evan Stade
https://codereview.chromium.org/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller.h File chrome/browser/ui/autofill/autofill_dialog_controller.h (right): https://codereview.chromium.org/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller.h#newcode51 chrome/browser/ui/autofill/autofill_dialog_controller.h:51: virtual std::pair<std::vector<string16>, string16> This definitely requires docs https://codereview.chromium.org/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller.h#newcode114 chrome/browser/ui/autofill/autofill_dialog_controller.h:114: ...
6 years, 5 months ago (2013-02-08 15:38:53 UTC) #2
Dan Beam
https://codereview.chromium.org/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller.h File chrome/browser/ui/autofill/autofill_dialog_controller.h (right): https://codereview.chromium.org/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller.h#newcode51 chrome/browser/ui/autofill/autofill_dialog_controller.h:51: virtual std::pair<std::vector<string16>, string16> On 2013/02/08 15:38:53, Evan Stade wrote: ...
6 years, 5 months ago (2013-02-08 20:53:05 UTC) #3
Dan Beam
hold off on this, making it work with multiple lengths of legal documents
6 years, 5 months ago (2013-02-08 22:41:51 UTC) #4
Dan Beam
OK, this should work with a semi-arbitrary number of legal documents now...
6 years, 5 months ago (2013-02-09 00:23:19 UTC) #5
Dan Beam
https://codereview.chromium.org/12225095/diff/7002/chrome/browser/ui/views/autofill/autofill_dialog_views.cc File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right): https://codereview.chromium.org/12225095/diff/7002/chrome/browser/ui/views/autofill/autofill_dialog_views.cc#newcode760 chrome/browser/ui/views/autofill/autofill_dialog_views.cc:760: text->SetHorizontalAlignment(gfx::ALIGN_LEFT); not sure if this is necessary https://codereview.chromium.org/12225095/diff/7002/chrome/browser/ui/views/autofill/autofill_dialog_views.cc#newcode766 chrome/browser/ui/views/autofill/autofill_dialog_views.cc:766: ...
6 years, 5 months ago (2013-02-09 00:27:57 UTC) #6
Dan Beam
https://chromiumcodereview.appspot.com/12225095/diff/5004/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right): https://chromiumcodereview.appspot.com/12225095/diff/5004/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc#newcode614 chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:614: #if defined(OS_ANDROID) So I guess Android is failing to ...
6 years, 5 months ago (2013-02-09 02:57:13 UTC) #7
Evan Stade
https://chromiumcodereview.appspot.com/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right): https://chromiumcodereview.appspot.com/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc#newcode997 chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:997: profile_, chrome::GetActiveDesktop()); On 2013/02/08 20:53:06, Dan Beam wrote: > ...
6 years, 5 months ago (2013-02-11 01:05:22 UTC) #8
Dan Beam
The rest require code change, can do tomorrow. https://chromiumcodereview.appspot.com/12225095/diff/5008/chrome/browser/ui/views/autofill/autofill_dialog_views.cc File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right): https://chromiumcodereview.appspot.com/12225095/diff/5008/chrome/browser/ui/views/autofill/autofill_dialog_views.cc#newcode723 chrome/browser/ui/views/autofill/autofill_dialog_views.cc:723: has_links ...
6 years, 5 months ago (2013-02-11 06:45:30 UTC) #9
ahutter
https://chromiumcodereview.appspot.com/12225095/diff/5004/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right): https://chromiumcodereview.appspot.com/12225095/diff/5004/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc#newcode308 chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:308: case 3U: Based in our conversations on Friday; this ...
6 years, 5 months ago (2013-02-11 16:00:53 UTC) #10
Dan Beam
https://chromiumcodereview.appspot.com/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right): https://chromiumcodereview.appspot.com/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc#newcode997 chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:997: profile_, chrome::GetActiveDesktop()); On 2013/02/11 01:05:23, Evan Stade wrote: > ...
6 years, 5 months ago (2013-02-11 16:45:17 UTC) #11
Dan Beam
https://chromiumcodereview.appspot.com/12225095/diff/5008/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right): https://chromiumcodereview.appspot.com/12225095/diff/5008/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc#newcode291 chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:291: DCHECK_LE(documents.size(), 3U); On 2013/02/11 16:00:53, ahutter wrote: > Should ...
6 years, 5 months ago (2013-02-11 18:30:05 UTC) #12
Dan Beam
https://codereview.chromium.org/12225095/diff/5004/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right): https://codereview.chromium.org/12225095/diff/5004/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc#newcode308 chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:308: case 3U: On 2013/02/11 16:00:53, ahutter wrote: > Based ...
6 years, 5 months ago (2013-02-11 19:27:50 UTC) #13
Dan Beam
done will all review feedback, sorry for early publishes https://chromiumcodereview.appspot.com/12225095/diff/5008/chrome/browser/ui/views/autofill/autofill_dialog_views.cc File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right): https://chromiumcodereview.appspot.com/12225095/diff/5008/chrome/browser/ui/views/autofill/autofill_dialog_views.cc#newcode488 chrome/browser/ui/views/autofill/autofill_dialog_views.cc:488: ...
6 years, 5 months ago (2013-02-11 19:45:59 UTC) #14
Dan Beam
+sky@ for ui/views, chrome/browser/ui/views
6 years, 5 months ago (2013-02-12 01:04:18 UTC) #15
Dan Beam
+sky@ for realz this time (for ui/views, chrome/browser/ui/views)
6 years, 5 months ago (2013-02-12 01:04:43 UTC) #16
Evan Stade
https://codereview.chromium.org/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right): https://codereview.chromium.org/12225095/diff/2001/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc#newcode997 chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:997: profile_, chrome::GetActiveDesktop()); On 2013/02/11 16:45:18, Dan Beam wrote: > ...
6 years, 5 months ago (2013-02-12 01:36:53 UTC) #17
Dan Beam
hey sky@, isherman@, estade@, ahutter@ - hold off on this until I figure out how ...
6 years, 5 months ago (2013-02-12 03:53:50 UTC) #18
Dan Beam
OK, this emulates text wrapping well enough for now -- PTAL (be gentle).
6 years, 5 months ago (2013-02-14 02:55:10 UTC) #19
Dan Beam
sky@/isherman@ ping (if you have time when not sheriffing, isherman@)
6 years, 5 months ago (2013-02-14 22:06:01 UTC) #20
sky
https://chromiumcodereview.appspot.com/12225095/diff/24012/chrome/browser/ui/views/autofill/autofill_dialog_views.h File chrome/browser/ui/views/autofill/autofill_dialog_views.h (right): https://chromiumcodereview.appspot.com/12225095/diff/24012/chrome/browser/ui/views/autofill/autofill_dialog_views.h#newcode276 chrome/browser/ui/views/autofill/autofill_dialog_views.h:276: class FootnoteView : public views::View { If I understand ...
6 years, 5 months ago (2013-02-14 22:47:10 UTC) #21
Dan Beam
https://chromiumcodereview.appspot.com/12225095/diff/24012/chrome/browser/ui/views/autofill/autofill_dialog_views.h File chrome/browser/ui/views/autofill/autofill_dialog_views.h (right): https://chromiumcodereview.appspot.com/12225095/diff/24012/chrome/browser/ui/views/autofill/autofill_dialog_views.h#newcode276 chrome/browser/ui/views/autofill/autofill_dialog_views.h:276: class FootnoteView : public views::View { On 2013/02/14 22:47:10, ...
6 years, 5 months ago (2013-02-15 01:48:31 UTC) #22
sky
Styled text should be a feature of the toolkit and be provided in views. It ...
6 years, 4 months ago (2013-02-19 21:59:58 UTC) #23
Dan Beam
On 2013/02/19 21:59:58, sky wrote: > Styled text should be a feature of the toolkit ...
6 years, 4 months ago (2013-02-20 23:19:31 UTC) #24
Dan Beam
On 2013/02/20 23:19:31, Dan Beam wrote: > On 2013/02/19 21:59:58, sky wrote: > > Styled ...
6 years, 4 months ago (2013-03-02 03:02:14 UTC) #25
sky
Sorry, I have had no time for this nor do I think I'll have any ...
6 years, 4 months ago (2013-03-04 21:42:28 UTC) #26
Evan Stade
6 years, 4 months ago (2013-03-11 21:25:04 UTC) #27
I will work on the rich label formatting.

Powered by Google App Engine
This is Rietveld 408576698