Closed Bug 148981 Opened 22 years ago Closed 22 years ago

STATE_TRANSFERRING not received for external stylesheets


(Core :: Networking: HTTP, defect)

Not set





(Reporter: KaiE, Assigned: rpotts)


(Blocks 1 open bug, )


(Keywords: topembed, Whiteboard: [adt2 rtm] [ETA 06/21])


(1 file)

The page refers to two external documents, index2.js and index2.css.

Expected behaviour:
Web Progress listeners receive OnStateChange events with STATE_TRANSFERRING for
each external document.

Actual behaviour:
Only for index2.js the STATE_TRANSFERRING event is received.
Not for index2.css
Blocks: 140836
I bet that external stylesheets doen't set up the proper notificationCallbacks...

i'll take a look.

-- rick
Getting on the Mach V radar.
Keywords: nsbeta1+, topembed
Whiteboard: [adt2 rtm]
rick - what are the chances we are gonna have a patch for this before monday?
Whiteboard: [adt2 rtm] → [adt2 rtm] [ETA Needed]
hey Kai,

here's a patch to make sure that STATE_TRANSFERRING notifications are fired
*even* if the nsIProgressEventSink notifications are not received...

after talking with you about the possibility of 'failed' requests STILL
delivering data... i added some (nasty) code to still fire the TRANSFERRING
notification for failed HTTP channels (if they were able to connect to the

let me know if you think this code makes sense... or if ONLY firing the
TRANSFERRING notification when the request succeeds is enough...

-- rick
I tried this patch, it works great!

Here is a testcase:
This is a secure page, but loads an insecure stylesheet.

With neither 140836 nor 148981 applied (current state of branch/trunk) go to
that URL. You will see the alert that you are viewing a page with mixed
encryption, as expected.

With having the patch from 140836 applied, we no longer see the mixed encryption
warning, but the page is incorrectly reported as secure.

With having both 140836 and 148981 applied, we again see the mixed encryption
alert, the required behaviour.

Rick, your patch is even better what I originally suggested. Instead of only
fixing stylesheets, your fix will help for any content. This makes a lot of
sense to my, and helps improve the lock icon reliability.

I looked at your patch. I think your additional check makes sense, and I'd
prefer the patch as you attached it. I think it is good to assume that data has
been transfered, even when only the initial server connection was made. This
won't hurt us, because when we have arrived in that state, the SSL handshake
will already have been completed, and we will be able to obtain the security
state for this channel, so we will make the correct decision. When in doubt, I
prefer receiving the TRANSFERRING more often than less.
Comment on attachment 88440 [details] [diff] [review]
Synthesize STATE_TRANSFERRING notifications if necessary


The logic in this patch makes sense to me.
(I understand everything but one line, which is:
  if ((oldMax == 0) && (info->mCurrentProgress == 0)) {
because I don't know the DocLoader internals.)
Attachment #88440 - Flags: review+
adt1.0.1+ (on adt's behalf) approval for checkin to the 1.0 branch, pending
drivers' approval. pls check this is asap, then add the "fixed1.0.1" keyword.
Whiteboard: [adt2 rtm] [ETA Needed] → [adt2 rtm] [ETA 06/21]
hey kai,

i added some more comments above the check of oldMax to clarify what's happening.

      // Determine whether a STATE_TRANSFERRING notification should be 
      // 'synthesized'.
      // If nsRequestInfo::mMaxProgress (as stored in oldMax) and
      // nsRequestInfo::mCurrentProgress are both 0, then the
      // STATE_TRANSFERRING notification has not been fired yet...

-- rick
Comment on attachment 88440 [details] [diff] [review]
Synthesize STATE_TRANSFERRING notifications if necessary


would be great to look into monitoring OnDataAvailable events from
nsDocumentOpenInfo if possible (as i mentioned to rick over the phone), but
perhaps that isn't possible... at any rate, this seems like a reasonable "hack"
for the short term.
Attachment #88440 - Flags: superreview+
I realized that hooking into the OnDataAvailable for the DoumentLoadInfo will
only work for top level documents...

Images, stylesheets, javascript, etc do NOT got through the URILoader :-(  So, i
think that for now, this is probably the best alternative...

-- rick
i've checked the patch into the trunk...
Blocks: 82458
rick: of course!! that makes sense. maybe a state attribute is really the way to
go. i smell nsIChannel2 :(
Blocks: 122150
Marking fixed on trunk.
Closed: 22 years ago
Resolution: --- → FIXED
Attachment #88440 - Flags: approval+
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Checked in for Rick to 1_0 branch.
verified trunk, verified1.0.1
Keywords: verified1.0.1
removing fixed1.0.1 keyword
Keywords: fixed1.0.1
You need to log in before you can comment on or make changes to this bug.