Loading remote stylesheet in editor fails

VERIFIED FIXED in M14

Status

()

P3
normal
VERIFIED FIXED
19 years ago
18 years ago

People

(Reporter: sfraser_bugs, Assigned: pierre)

Tracking

Trunk
All
Mac System 8.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: Release note for M12)

(Reporter)

Description

19 years ago
In the editor, choose Format -> Apply Style Sheet -> Oldstyle (or one of the
other W3C style sheets at the bottom of the submenu.

On Mac, it asserts "No response" in nsHTTPChannel::OpenInputStream(), then
crashes the machine hard. On Linux, a different crash happens.

This all worked fine in the old netlib.

Comment 1

19 years ago
On Linux (RH6.0), I also see a crash, but the stack trace is a little different:

#0  0x4033aac9 in ConverterInputStream::ConverterInputStream (this=0x86817d0,
    aStream=0x40a16a2c, aConverter=0x884ccf0, aBufferSize=0)
    at nsUnicharInputStream.cpp:189
#1  0x4033b100 in NS_NewConverterStream (aInstancePtrResult=0xbfffd34c,
    aOuter=0x0, aStreamToWrap=0x40a16a2c, aBufferSize=0, aCharSet=0x0)
    at nsUnicharInputStream.cpp:312
#2  0x40f66485 in CSSLoaderImpl::LoadAgentSheet (this=0x8579fc8,
    aURL=0x884cfe0, aSheet=@0xbfffd3ac, aCompleted=@0xbfffd3a8,
    aCallback=0x41dae168
<nsHTMLEditor::ApplyStyleSheetToPresShellDocument(nsICSSStyleSheet *, void *)>,
aData=0x86153a0) at nsCSSLoader.cpp:1308
#3  0x41dab2f5 in nsHTMLEditor::ApplyStyleSheet (this=0x86153a0,
    aURL=@0xbfffd408) at nsHTMLEditor.cpp:2586

Comment 2

19 years ago
Simon, do we need this for M9
(Reporter)

Updated

19 years ago
Whiteboard: Release note for M9
Target Milestone: M10
(Reporter)

Comment 3

19 years ago
This should be release-noted for M9.

Comment 4

19 years ago
K, I added it to bug 11352, RN tracker

Updated

19 years ago
Status: NEW → ASSIGNED
Target Milestone: M10 → M12

Comment 5

19 years ago
OpenInputStream is still a big black hole. See bug 10456.

This happens now on linux:

CSSLoaderImpl::LoadAgentSheet: Load of URL
'http://www.w3.org/StyleSheets/Core/Steely' failed.  Error code: 16385
JavaScript Error: uncaught exception: [Exception... "Component returned failure
code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIEditorShell.ApplyStyleSheet]"
nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame ::
chrome://editor/content/EditorCommands.js :: EditorApplyStyleSheet :: line 839"
data: no]

Comment 6

19 years ago
Moving Assignee from gagan to warren since he is away.

Updated

19 years ago
Assignee: warren → sfraser

Comment 7

19 years ago
I can't get anything to happen when I try to apply a style sheet. Is this still
hooked up? Note that we are still waiting on OpenInputStream to be implemented
(however that may affect this bug). Reassigning back to Simon for reevaluation.
(Reporter)

Updated

19 years ago
Status: NEW → ASSIGNED
Summary: Crash loading remote stylesheet in editor → Loading remote stylesheet in editor fails
(Reporter)

Comment 8

19 years ago
This is not a crash, just a failure. Fixing summary.
(Reporter)

Updated

19 years ago
Depends on: 18434
Target Milestone: M12 → M13
(Reporter)

Comment 9

19 years ago
Yes, this is failing because CSSLoaderImpl::LoadAgentSheet() calls
NS_OpenURI(nsIInputStream **...) which calls nsHTTPChannel::OpenInputStream()
which is not implemented.

Marking dependent on 18434, moving out.

Comment 10

19 years ago
Bulk move of all Necko (to be deleted component) bugs to new Networking

component.
(Reporter)

Comment 11

19 years ago
*** Bug 21941 has been marked as a duplicate of this bug. ***

Updated

19 years ago
Assignee: sfraser → valeski
Status: ASSIGNED → NEW

Comment 12

19 years ago
From Pierre Saslawsky concerning bug 21941:
"It freezes in nsPipe: this is an I/O or a threading issue. I'm going to
reassign the bug to the Necko people."
Since 21941 was marked a duplicate of this, I'm reassigning to network group.
Look at 21941 for more stack info if needed.
(Reporter)

Updated

19 years ago
Whiteboard: Release note for M9 → Release note for M12
(Reporter)

Comment 13

19 years ago
I think the underlying problem is as described in bug 18434; there is some
threading badness with nsHTTPChannel::OpenInputStream().

Comment 14

19 years ago
cc'ing potts.

Comment 15

19 years ago
Current protocol implementations of OpenInputStream() require that the returned
stream be read on a seperate thread. Current implementations simply call
AsyncRead() underneath. Can the editor use AsyncRead() instead?

Updated

19 years ago
Depends on: 22103
No longer depends on: 18434
(Reporter)

Updated

19 years ago
Assignee: valeski → pierre
(Reporter)

Comment 16

19 years ago
Editor is not doing the reading, the style system is. Reassigning to pierre.
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

19 years ago
Assignee: pierre → valeski
Status: ASSIGNED → NEW
(Assignee)

Comment 17

19 years ago
As described by cmanske in bug 21841, the reading is done somewhere way
underneath the style system. The stack shows:

nsCSSScanner::Read(int&)
ConverterInputStream::Read(unsigned short*,unsigned int,unsigned int,unsigned
int*)
ConverterInputStream::Fill(unsigned int*)
ByteBufferImpl::Fill(unsigned int*,nsIInputStream*,unsigned int)
nsPipe::nsPipeInputStream::Read(char*,unsigned int,unsigned int*)
nsPipe::nsPipeInputStream::Fill()

Then it freezes in nsAutoCMonitor::Wait(). None of these objects implements
AsyncRead(). I hate to do that but I have to reassign to Valeski again.

Comment 18

19 years ago
So who's initiating the load? Whoever is doing that needs to use
AsyncRead() instead. OpenInputStream() blocks, we can't block the main thread.
I"m not sure what you meain by "none of htese objects implements asyncread", all
the network protocols do.

Comment 19

19 years ago
The load is initiated in CSSLoaderImpl::LoadAgentSheet() (as pointed out above).
The CSS loader should be changed to read data asyncronously, and the unichar
stream converter should implement the nsIStreamConverter interface to handle the
async data conversion.

Is this code called in the browser? If so this is probably a thread blocker
(thus time sink).
(Assignee)

Comment 20

19 years ago
I'm afraid it works that way by design: the agent sheet must be loaded
synchronously. What can we do to make it work without blocking the thread
forever?

Comment 21

19 years ago
FYI : This function works for me with a local stylesheet, but only once.  Trying
it a second time fails, but with no errors returned or no crashes.

Comment 22

19 years ago
By definition, synchronous == blocking, which is a bad thing in a GUI app. We're
investigating a re-write of the OpenInputStream calls on the necko side. It's a
big and lengthy change though.

I'll take a look at re-writing the CSS agent stuff, and accompanying converter,
to do the right thing.

Updated

19 years ago
Status: NEW → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → WONTFIX

Comment 23

19 years ago
blocking reads are illegal on the UI thread. I don't have time to rework the
reader to go async.

Comment 24

19 years ago
how come we're not gonna fix this? it freezes the editor...
(Reporter)

Comment 25

19 years ago
pierre will have to fix stylesheet loading to be async.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
(Reporter)

Comment 26

19 years ago
Reassign to pierre
Assignee: valeski → pierre
Status: REOPENED → NEW

Updated

19 years ago
Target Milestone: M13 → M14

Comment 27

19 years ago
Taking over 1/3 of Pierre's NEW bugs to help reduce his doomage factor
Assignee: pierre → attinasi
(Assignee)

Comment 28

19 years ago
Reassigned back to me these bugs that shouldn't have left my list.
Assignee: attinasi → pierre
(Assignee)

Comment 29

19 years ago
Fix checked in nsCSSLoader.cpp. The command has been re-enabled in 
editorOverlay.xul. The editor folks will be happy again: it's quite fun to play 
with that command. I'm sorry it was broken for so long.

FYI, I opened two related bugs: bug 32069 and bug 32070.
Status: NEW → RESOLVED
Last Resolved: 19 years ago19 years ago
Resolution: --- → FIXED

Comment 30

19 years ago
qa to sujay
QA Contact: paulmac → sujay

Updated

19 years ago
No longer depends on: 22103

Comment 31

18 years ago
verified in 7/25 build
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.