Last Comment Bug 704691 - Fire STATE_IS_NETWORK notifications with print status so that the download manager can track "Save as PDF" prints
: Fire STATE_IS_NETWORK notifications with print status so that the download ma...
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla11
Assigned To: :Margaret Leibovic
: Jet Villegas (:jet)
Depends on: 704840 732382
Blocks: 701797
  Show dependency treegraph
Reported: 2011-11-22 17:27 PST by :Margaret Leibovic
Modified: 2012-04-19 16:57 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (1.38 KB, patch)
2011-11-22 17:27 PST, :Margaret Leibovic
bzbarsky: review-
Details | Diff | Splinter Review
patch v2 (2.74 KB, patch)
2011-11-27 22:00 PST, :Margaret Leibovic
bzbarsky: review+
Details | Diff | Splinter Review

Description User image :Margaret Leibovic 2011-11-22 17:27:38 PST
Created attachment 576358 [details] [diff] [review]

From bug 701797 comment 9:

In order to use the download manager to track printing status, we need to fire STATE_IS_NETWORK notifications, since that's what the download manager expects. I'm not sure if this fix is hacky or not, but it doesn't look like it will cause any trouble.
Comment 1 User image :Margaret Leibovic 2011-11-22 19:42:12 PST
(Note: I'm planning on closing bug 701797 and obsoleting the version of the patch in there, but thanks to bug 704699 I can't do that yet!)
Comment 2 User image Boris Zbarsky [:bz] (still a bit busy) 2011-11-23 08:34:16 PST
Comment on attachment 576358 [details] [diff] [review]

Looking at how docloader does this:

1)  The STATE_STOP for STATE_IS_DOCUMENT comes before STATE_IS_NETWORK.  I believe the API actually requires this.

2)  The STATE_START for STATE_IS_NETWORK comes at the same time as STATE_IS_DOCUMENT.  That is, there is a single call with STATE_START|STATE_IS_DOCUMENT|STATE_IS_NETWORK as the flags.

We definitely need to do #1, and I think it's a good idea to do #2 as well.
Comment 3 User image :Margaret Leibovic 2011-11-27 22:00:46 PST
Created attachment 577196 [details] [diff] [review]
patch v2

I also saw that the STATE_START wasn't being fired anyway because of a check in DoOnProgressChange. I decided to remove that check, since the only caller of DoOnProgressChange that passes 0 for aProgress is the call in OnStartPrinting, and we don't actually want that one to be ignored. (I also fixed a s/progess/progress/ typo.)
Comment 4 User image Armen Zambrano - Back on March 27th [:armenzg] (EDT/UTC-4) 2011-11-28 08:33:06 PST
I filed bug 705699 to don't clutter this one and to make it easier for people to find it in queries.
Comment 5 User image Boris Zbarsky [:bz] (still a bit busy) 2011-11-28 12:44:01 PST
Comment on attachment 577196 [details] [diff] [review]
patch v2

Comment 6 User image :Margaret Leibovic 2011-11-28 13:49:58 PST
Pushed to inbound:
Comment 7 User image Marco Bonardo [::mak] 2011-11-28 16:15:31 PST
I backed out and relanded this, since the original push had some additional unicode char in the commit message, and that confused buildbot sendchange
Comment 8 User image Marco Bonardo [::mak] 2011-11-29 04:59:17 PST

Note You need to log in before you can comment on or make changes to this bug.