|
|
Chromium Code Reviews|
Created:
8 years, 3 months ago by Vitaly Buka (NO REVIEWS) Modified:
8 years, 3 months ago CC:
chromium-reviews, erikwright+watch_chromium.org Base URL:
http://git.chromium.org/chromium/src.git@page Visibility:
Public. |
DescriptionMatch paper by both PPD name and size.
According http://lists.apple.com/archives/printing/2011/Nov/msg00001.html PMPaperGetPPDPaperName should provide better result than just size.
BUG=145237
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=155783
Patch Set 1 : #Patch Set 2 : #
Total comments: 19
Patch Set 3 : #
Total comments: 5
Patch Set 4 : #Messages
Total messages: 23 (0 generated)
More conservative approach to fix the issue. I still like to find use-cases where this function important.
If we don't really understand the cases where this was originally needed, can we add tests here to recreate them and make sure we're not breaking them?
+1, I'd rather go with other CL unless we have good explanation. If we break something, we will receive feedback with more info. On 2012/09/07 17:17:43, Albert Bodenhamer wrote: > If we don't really understand the cases where this was originally needed, can we > add tests here to recreate them and make sure we're not breaking them?
My point is I want to understand BEFORE we break something. On Fri, Sep 7, 2012 at 11:24 AM, <vitalybuka@chromium.org> wrote: > +1, I'd rather go with other CL unless we have good explanation. > If we break something, we will receive feedback with more info. > > > On 2012/09/07 17:17:43, Albert Bodenhamer wrote: > >> If we don't really understand the cases where this was originally needed, >> can >> > we > >> add tests here to recreate them and make sure we're not breaking them? >> > > > > http://chromiumcodereview.**appspot.com/10918117/<http://chromiumcodereview.a... > -- Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com> org
On 2012/09/07 18:33:40, Albert Bodenhamer wrote: > My point is I want to understand BEFORE we break something. > > > On Fri, Sep 7, 2012 at 11:24 AM, <mailto:vitalybuka@chromium.org> wrote: > > > +1, I'd rather go with other CL unless we have good explanation. > > If we break something, we will receive feedback with more info. > > > > > > On 2012/09/07 17:17:43, Albert Bodenhamer wrote: > > > >> If we don't really understand the cases where this was originally needed, > >> can > >> > > we > > > >> add tests here to recreate them and make sure we're not breaking them? > >> > > > > > > > > > http://chromiumcodereview.**appspot.com/10918117/%3Chttp://chromiumcodereview...> > > > > > > -- > Albert Bodenhamer | Software Engineer | mailto:abodenha@chromium.<abodenha@google.com> > org I did some manual testing and added my comments in the previous CL https://chromiumcodereview.appspot.com/10918109/ for reference. Please take a look and let me know your comments.
On 2012/09/07 18:33:40, Albert Bodenhamer wrote: > My point is I want to understand BEFORE we break something. It's in ideal world. Now we already in broken state, and we have to do something with or without understanding. For the second case I prefer just to reduce complexity. > > > On Fri, Sep 7, 2012 at 11:24 AM, <mailto:vitalybuka@chromium.org> wrote: > > > +1, I'd rather go with other CL unless we have good explanation. > > If we break something, we will receive feedback with more info. > > > > > > On 2012/09/07 17:17:43, Albert Bodenhamer wrote: > > > >> If we don't really understand the cases where this was originally needed, > >> can > >> > > we > > > >> add tests here to recreate them and make sure we're not breaking them? > >> > > > > > > > > > http://chromiumcodereview.**appspot.com/10918117/%3Chttp://chromiumcodereview...> > > > > > > -- > Albert Bodenhamer | Software Engineer | mailto:abodenha@chromium.<abodenha@google.com> > org
On Fri, Sep 7, 2012 at 12:58 PM, <vitalybuka@chromium.org> wrote: > On 2012/09/07 18:33:40, Albert Bodenhamer wrote: > >> My point is I want to understand BEFORE we break something. >> > > It's in ideal world. > Now we already in broken state, and we have to do something with or without > understanding. For the second case I prefer just to reduce complexity. > > That's exactly how we ended up with crbug.com/144048. I'm fine going with the simpler solution, but I'd feel better if we had more unit tests. > > > > On Fri, Sep 7, 2012 at 11:24 AM, <mailto:vitalybuka@chromium.**org<vitalybuka@chromium.org>> >> wrote: >> > > > +1, I'd rather go with other CL unless we have good explanation. >> > If we break something, we will receive feedback with more info. >> > >> > >> > On 2012/09/07 17:17:43, Albert Bodenhamer wrote: >> > >> >> If we don't really understand the cases where this was originally >> needed, >> >> can >> >> >> > we >> > >> >> add tests here to recreate them and make sure we're not breaking them? >> >> >> > >> > >> > >> > >> > > http://chromiumcodereview.**ap**pspot.com/10918117/%3Chttp://** > chromiumcodereview.appspot.**com/10918117/<http://appspot.com/10918117/%3Chttp://chromiumcodereview.appspot.com/10918117/> > > > >> > >> > > > > -- >> Albert Bodenhamer | Software Engineer | >> > mailto:abodenha@chromium.<abod**enha@google.com <abodenha@google.com>> > >> org >> > > > > http://chromiumcodereview.**appspot.com/10918117/<http://chromiumcodereview.a... > -- Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com> org
On Fri, Sep 7, 2012 at 1:04 PM, Albert Bodenhamer <abodenha@chromium.org>wrote: > > On Fri, Sep 7, 2012 at 12:58 PM, <vitalybuka@chromium.org> wrote: > >> On 2012/09/07 18:33:40, Albert Bodenhamer wrote: >> >>> My point is I want to understand BEFORE we break something. >>> >> >> It's in ideal world. >> Now we already in broken state, and we have to do something with or >> without >> understanding. For the second case I prefer just to reduce complexity. >> >> > That's exactly how we ended up with crbug.com/144048. > That was different, we had no feeling that we didn't understand something :-) > > I'm fine going with the simpler solution, but I'd feel better if we had > more unit tests. > You mean tests, even manual, I don't think issue that was addressed by original CL was unit-testable. I'll try rollback to the point when this patch was introduces and see how to reproduce issue. Problem that it's time consuming and does not guaranty success. > >> >> >> >> On Fri, Sep 7, 2012 at 11:24 AM, <mailto:vitalybuka@chromium.**org<vitalybuka@chromium.org>> >>> wrote: >>> >> >> > +1, I'd rather go with other CL unless we have good explanation. >>> > If we break something, we will receive feedback with more info. >>> > >>> > >>> > On 2012/09/07 17:17:43, Albert Bodenhamer wrote: >>> > >>> >> If we don't really understand the cases where this was originally >>> needed, >>> >> can >>> >> >>> > we >>> > >>> >> add tests here to recreate them and make sure we're not breaking them? >>> >> >>> > >>> > >>> > >>> > >>> >> >> http://chromiumcodereview.**ap**pspot.com/10918117/%3Chttp://** >> chromiumcodereview.appspot.**com/10918117/<http://appspot.com/10918117/%3Chttp://chromiumcodereview.appspot.com/10918117/> >> > >> >>> > >>> >> >> >> >> -- >>> Albert Bodenhamer | Software Engineer | >>> >> mailto:abodenha@chromium.<abod**enha@google.com <abodenha@google.com>> >> >>> org >>> >> >> >> >> http://chromiumcodereview.**appspot.com/10918117/<http://chromiumcodereview.a... >> > > > > -- > Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com> > org >
On Fri, Sep 7, 2012 at 1:19 PM, Vitaly Buka <vitalybuka@chromium.org> wrote: > On Fri, Sep 7, 2012 at 1:04 PM, Albert Bodenhamer <abodenha@chromium.org>wrote: > >> >> On Fri, Sep 7, 2012 at 12:58 PM, <vitalybuka@chromium.org> wrote: >> >>> On 2012/09/07 18:33:40, Albert Bodenhamer wrote: >>> >>>> My point is I want to understand BEFORE we break something. >>>> >>> >>> It's in ideal world. >>> Now we already in broken state, and we have to do something with or >>> without >>> understanding. For the second case I prefer just to reduce complexity. >>> >>> >> That's exactly how we ended up with crbug.com/144048. >> > > That was different, we had no feeling that we didn't understand something > :-) > > >> >> I'm fine going with the simpler solution, but I'd feel better if we had >> more unit tests. >> > > You mean tests, even manual, I don't think issue that was addressed by > original CL was unit-testable. > I'll try rollback to the point when this patch was introduces and see how > to reproduce issue. Problem that it's time consuming and does not guaranty > success. > OK. If you're confident, let's go with the simpler patch We really need to figure out a better way to test these things in the future. :-/ > > >> >>> >>> >>> >>> On Fri, Sep 7, 2012 at 11:24 AM, <mailto:vitalybuka@chromium.**org<vitalybuka@chromium.org>> >>>> wrote: >>>> >>> >>> > +1, I'd rather go with other CL unless we have good explanation. >>>> > If we break something, we will receive feedback with more info. >>>> > >>>> > >>>> > On 2012/09/07 17:17:43, Albert Bodenhamer wrote: >>>> > >>>> >> If we don't really understand the cases where this was originally >>>> needed, >>>> >> can >>>> >> >>>> > we >>>> > >>>> >> add tests here to recreate them and make sure we're not breaking >>>> them? >>>> >> >>>> > >>>> > >>>> > >>>> > >>>> >>> >>> http://chromiumcodereview.**ap**pspot.com/10918117/%3Chttp://** >>> chromiumcodereview.appspot.**com/10918117/<http://appspot.com/10918117/%3Chttp://chromiumcodereview.appspot.com/10918117/> >>> > >>> >>>> > >>>> >>> >>> >>> >>> -- >>>> Albert Bodenhamer | Software Engineer | >>>> >>> mailto:abodenha@chromium.<abod**enha@google.com <abodenha@google.com>> >>> >>>> org >>>> >>> >>> >>> >>> http://chromiumcodereview.**appspot.com/10918117/<http://chromiumcodereview.a... >>> >> >> >> >> -- >> Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com> >> org >> > > -- Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com> org
On 2012/09/07 20:24:25, Albert Bodenhamer wrote: > On Fri, Sep 7, 2012 at 1:19 PM, Vitaly Buka <mailto:vitalybuka@chromium.org> wrote: > > > On Fri, Sep 7, 2012 at 1:04 PM, Albert Bodenhamer > <abodenha@chromium.org>wrote: > > > >> > >> On Fri, Sep 7, 2012 at 12:58 PM, <mailto:vitalybuka@chromium.org> wrote: > >> > >>> On 2012/09/07 18:33:40, Albert Bodenhamer wrote: > >>> > >>>> My point is I want to understand BEFORE we break something. > >>>> > >>> > >>> It's in ideal world. > >>> Now we already in broken state, and we have to do something with or > >>> without > >>> understanding. For the second case I prefer just to reduce complexity. > >>> > >>> > >> That's exactly how we ended up with crbug.com/144048. > >> > > > > That was different, we had no feeling that we didn't understand something > > :-) > > > > > >> > >> I'm fine going with the simpler solution, but I'd feel better if we had > >> more unit tests. > >> > > > > You mean tests, even manual, I don't think issue that was addressed by > > original CL was unit-testable. > > I'll try rollback to the point when this patch was introduces and see how > > to reproduce issue. Problem that it's time consuming and does not guaranty > > success. > > > > OK. If you're confident, let's go with the simpler patch We really need > to figure out a better way to test these things in the future. :-/ > > > > > > > >> > >>> > >>> > >>> > >>> On Fri, Sep 7, 2012 at 11:24 AM, > <mailto:vitalybuka@chromium.**org<vitalybuka@chromium.org>> > >>>> wrote: > >>>> > >>> > >>> > +1, I'd rather go with other CL unless we have good explanation. > >>>> > If we break something, we will receive feedback with more info. > >>>> > > >>>> > > >>>> > On 2012/09/07 17:17:43, Albert Bodenhamer wrote: > >>>> > > >>>> >> If we don't really understand the cases where this was originally > >>>> needed, > >>>> >> can > >>>> >> > >>>> > we > >>>> > > >>>> >> add tests here to recreate them and make sure we're not breaking > >>>> them? > >>>> >> > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > >>> > >>> http://chromiumcodereview.**ap**pspot.com/10918117/%253Chttp://** > >>> > chromiumcodereview.appspot.**com/10918117/<http://appspot.com/10918117/%3Chttp://chromiumcodereview.appspot.com/10918117/> > >>> > > >>> > >>>> > > >>>> > >>> > >>> > >>> > >>> -- > >>>> Albert Bodenhamer | Software Engineer | > >>>> > >>> mailto:abodenha@chromium.<abod**enha@google.com <mailto:abodenha@google.com>> > >>> > >>>> org > >>>> > >>> > >>> > >>> > >>> > http://chromiumcodereview.**appspot.com/10918117/%3Chttp://chromiumcodereview...> > >>> > >> > >> > >> > >> -- > >> Albert Bodenhamer | Software Engineer | > mailto:abodenha@chromium.<abodenha@google.com> > >> org > >> > > > > > > > -- > Albert Bodenhamer | Software Engineer | mailto:abodenha@chromium.<abodenha@google.com> > org I am deferring this review until we understand the original problem and I don't think it is a good idea to find the matching paper based on ppd paper names. Printers can have custom paper names in ppd files. There is a chance that we will fail to find the right matching paper. If this bug fix can wait, I can try to reproduce the original issue after M23 deadline. Thanks.
On 2012/09/07 21:18:22, kmadhusu wrote: > I am deferring this review until we understand the original problem and I don't > think it is a good idea to find the matching paper based on ppd paper names. > Printers can have custom paper names in ppd files. There is a chance that we > will fail to find the right matching paper. If this bug fix can wait, I can try > to reproduce the original issue after M23 deadline. > > Thanks. There is a chance with current algorithm too. But we know for sure that matching by size only is not acceptable, maybe (name && size). Also Apple developer, in mentioned mailing list, states that this names should be consistent. It can wait, but I spent some time and I like to finish without losing context. I don't expect from you investment of much time. Maybe your last big comment on another issue is enough.
On 2012/09/07 21:58:34, Vitaly Buka wrote: > On 2012/09/07 21:18:22, kmadhusu wrote: > > I am deferring this review until we understand the original problem and I > don't > > think it is a good idea to find the matching paper based on ppd paper names. > > Printers can have custom paper names in ppd files. There is a chance that we > > will fail to find the right matching paper. If this bug fix can wait, I can > try > > to reproduce the original issue after M23 deadline. > > > > Thanks. > > > There is a chance with current algorithm too. But we know for sure that matching > by size only is not acceptable, maybe (name && size). Also Apple developer, in > mentioned mailing list, states that this names should be consistent. > > It can wait, but I spent some time and I like to finish without losing context. > I don't expect from you investment of much time. Maybe your last big comment on > another issue is enough. +1 for checking both name and size. Let me know once this CL is ready for review.
Just tried another printer, Epson NX530, with current chrome. Scaling is wrong, text quality is bad. Just because LetterFullBleed used instead of Letter. On Fri, Sep 7, 2012 at 3:21 PM, <kmadhusu@chromium.org> wrote: > On 2012/09/07 21:58:34, Vitaly Buka wrote: > >> On 2012/09/07 21:18:22, kmadhusu wrote: >> > I am deferring this review until we understand the original problem and >> I >> don't >> > think it is a good idea to find the matching paper based on ppd paper >> names. >> > Printers can have custom paper names in ppd files. There is a chance >> that we >> > will fail to find the right matching paper. If this bug fix can wait, I >> can >> try >> > to reproduce the original issue after M23 deadline. >> > >> > Thanks. >> > > > There is a chance with current algorithm too. But we know for sure that >> > matching > >> by size only is not acceptable, maybe (name && size). Also Apple >> developer, in >> mentioned mailing list, states that this names should be consistent. >> > > It can wait, but I spent some time and I like to finish without losing >> > context. > >> I don't expect from you investment of much time. Maybe your last big >> comment >> > on > >> another issue is enough. >> > > +1 for checking both name and size. Let me know once this CL is ready for > review. > > https://chromiumcodereview.**appspot.com/10918117/<https://chromiumcodereview... >
Please review.
https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... File printing/printing_context_mac.mm (right): https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:25: std::string PaperToString(const PMPaper paper) { Document this function. https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:25: std::string PaperToString(const PMPaper paper) { const PMPaper -> const PMPaper& https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:29: std::string result(CFStringGetLength(name) + 1, ' '); Use base::mac::ScopedCFTypeRef<CFStringRef> https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:38: bool IsPaperNameEqual(const PMPaper paper1, const PMPaper paper2) { nit: Document this function. https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:39: CFStringRef name1 = NULL; Use base::mac::ScopedCFTypeRef<CFStringRef> https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:41: return PMPaperGetPPDPaperName(paper1, &name1) == noErr && nit: Please add parantheses. return (a == b) && ( e == b) && (c == d); https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:267: std::setprecision(10) << default_page_width << "x" << Do you need these VLOG here and at 290 and 324? https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:293: double new_score = std::max(fabs(default_page_width - paper_width), nit: Can you rename new_score to current_match or something similar? https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:293: double new_score = std::max(fabs(default_page_width - paper_width), Please re-add the comments that explain the paper order priority.
https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... File printing/printing_context_mac.mm (right): https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:25: std::string PaperToString(const PMPaper paper) { Don't need if remove logging. On 2012/09/08 01:06:16, kmadhusu wrote: > Document this function. https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:25: std::string PaperToString(const PMPaper paper) { On 2012/09/08 01:06:16, kmadhusu wrote: > const PMPaper -> const PMPaper& Done. https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:38: bool IsPaperNameEqual(const PMPaper paper1, const PMPaper paper2) { On 2012/09/08 01:06:16, kmadhusu wrote: > nit: Document this function. Done. https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:39: CFStringRef name1 = NULL; I am not experienced with MAC dev, but it seems we don't need Retain/Release here On 2012/09/08 01:06:16, kmadhusu wrote: > Use base::mac::ScopedCFTypeRef<CFStringRef> https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:41: return PMPaperGetPPDPaperName(paper1, &name1) == noErr && On 2012/09/08 01:06:16, kmadhusu wrote: > nit: Please add parantheses. > return (a == b) && ( e == b) && (c == d); Done. https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:267: std::setprecision(10) << default_page_width << "x" << On 2012/09/08 01:06:16, kmadhusu wrote: > Do you need these VLOG here and at 290 and 324? Done. https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:293: double new_score = std::max(fabs(default_page_width - paper_width), On 2012/09/08 01:06:16, kmadhusu wrote: > nit: Can you rename new_score to current_match or something similar? Done. https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:293: double new_score = std::max(fabs(default_page_width - paper_width), It's not relevant here. I removed the check for 0.0001 Now it's basically your function with "+= IsPaperNameEqual()" On 2012/09/08 01:06:16, kmadhusu wrote: > Please re-add the comments that explain the paper order priority. https://chromiumcodereview.appspot.com/10918117/diff/11002/printing/printing_... File printing/printing_context_mac.mm (right): https://chromiumcodereview.appspot.com/10918117/diff/11002/printing/printing_... printing/printing_context_mac.mm:280: current_match += IsPaperNameEqual(paper, default_paper) ? 0 : 1; Without line 280 it should work exactly as pre-CL version.
lgtm. I am not a mac developer. So, adding csilv@ to take a quick look. Please wait for csilv@ lgtm. Thanks.
https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... File printing/printing_context_mac.mm (right): https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:39: CFStringRef name1 = NULL; On 2012/09/08 01:54:19, Vitaly Buka wrote: > I am not experienced with MAC dev, but it seems we don't need Retain/Release > here > On 2012/09/08 01:06:16, kmadhusu wrote: > > Use base::mac::ScopedCFTypeRef<CFStringRef> > csilv@: Do we need to use base::mac::ScopedCFTypeRef<CFStringRef> here?
LGTM https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... File printing/printing_context_mac.mm (right): https://chromiumcodereview.appspot.com/10918117/diff/1007/printing/printing_c... printing/printing_context_mac.mm:39: CFStringRef name1 = NULL; On 2012/09/08 02:26:18, kmadhusu wrote: > On 2012/09/08 01:54:19, Vitaly Buka wrote: > > I am not experienced with MAC dev, but it seems we don't need Retain/Release > > here > > On 2012/09/08 01:06:16, kmadhusu wrote: > > > Use base::mac::ScopedCFTypeRef<CFStringRef> > > > csilv@: Do we need to use base::mac::ScopedCFTypeRef<CFStringRef> here? kmadhusu@, no reason to use ScopedCFTypeRef here. PMPaperGetPPDPaperName retains ownership of the returned object. So there is no reason for the object to be released and therefore no real benefit of ScopedCFTypeRef. https://chromiumcodereview.appspot.com/10918117/diff/11002/printing/printing_... File printing/printing_context_mac.mm (right): https://chromiumcodereview.appspot.com/10918117/diff/11002/printing/printing_... printing/printing_context_mac.mm:270: PMPaper paper = (PMPaper) [(NSArray* ) paper_list objectAtIndex: i]; nit: remove random whitespace making it: PMPaper paper = (PMPaper)[(NSArray*)paper_list objectAtIndex: i]; https://chromiumcodereview.appspot.com/10918117/diff/11002/printing/printing_... printing/printing_context_mac.mm:277: // Ignore paper that very different. nit: "Ignore paper sizes that are very different."
lgtm
https://chromiumcodereview.appspot.com/10918117/diff/11002/printing/printing_... File printing/printing_context_mac.mm (right): https://chromiumcodereview.appspot.com/10918117/diff/11002/printing/printing_... printing/printing_context_mac.mm:270: PMPaper paper = (PMPaper) [(NSArray* ) paper_list objectAtIndex: i]; On 2012/09/10 03:27:47, csilv wrote: > nit: remove random whitespace making it: > PMPaper paper = (PMPaper)[(NSArray*)paper_list objectAtIndex: i]; Done. https://chromiumcodereview.appspot.com/10918117/diff/11002/printing/printing_... printing/printing_context_mac.mm:277: // Ignore paper that very different. On 2012/09/10 03:27:47, csilv wrote: > nit: "Ignore paper sizes that are very different." Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/10918117/14004
Change committed as 155783 |
