558742 - Comma doesn't work in print range

Status: RESOLVED FIXED

(Toolkit :: Printing, defect)

Description

[vicky.sun]

Mozilla/5.0 (X11; U; SunOS i86pc; en-US; rv:1.9.1.9) Gecko/20100406 Firefox/3.5.9

It can also be reproduced FF3.5.6 on Ubuntu 9.10, FF3.6 on OpenSolaris

Steps to reproduce:
1. Launch FF and load a webpage, e.g. www.mozillazine.org
2. Go to File -> Print -> Range -> Pages, enter 1,3,5, then click Print button

Actual result: Page 1 to 5 are printed
Expected result: Page 1,3 and 5 are printed

Comment 2

[[not reading bugmail]]

Any reason why this would be the same on each platform?

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Attachment: printing ranges implementation

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 577277 [details] [diff] [review]
printing ranges implementation

I wonder who is the best reviewer for it, please reassign if you're the wrong one.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 577277 [details] [diff] [review]
printing ranges implementation

Review of attachment 577277 [details] [diff] [review]:
-----------------------------------------------------------------

Also, rev UUID on nsIPrintSettings.idl.

::: widget/public/nsIPrintSettings.idl
@@ +333,5 @@
>    [noscript] void GetUnwriteableMarginInTwips(in nsNativeIntMarginRef aEdge);
> +  
> +  /**
> +   * Get more accurate printing ranges
> +   */

Please document this! Need to at least mention that the array is a list of (start, end) pairs where the endpoints are inclusive. Also need to specify that ranges can't overlap.

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Attachment: printing ranges implementation, v2

Patch in commit format, new UUID for nsIPrintSettings, updated doc. Carry on r+ from roc. Is sr needed for this one?

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 577912 [details] [diff] [review]
printing ranges implementation, v2

According to http://www.mozilla.org/hacking/reviewers.html is sr requested (API change, cross-module). Boris, can you look at it please?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 577912 [details] [diff] [review]
printing ranges implementation, v2

> +      // Page ranges are couples

Maybe "pairs"?  And use "%" to make it clearer what's being tested?

>+  [noscript] void GetPageRanges(out IntegerArray aPages);

This isn't right.  The contract for that IDL is that GetPageRanges allocates an array and puts it at the provided pointer.

What you want to do is something more like what GetUnwriteableMarginInTwips does: declare a ref-to-nsTArray type and use an in parameter of that type.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

Attachment: printing ranges implementation, v3

Thanks Boris, this one should fix it.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 579052 [details] [diff] [review]
printing ranges implementation, v3

"length % 2 == 0" is probably clearer than using !.  With that change, sr=me.  Thanks!

Comment 11

[Martin Stránský [:stransky] (ni? me)]

Attachment: v4

Thanks, there's the "length % 2 == 0" version here.

Comment 12

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/845129faeace

Comment 13

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/845129faeace