Closed Bug 14599 Opened 21 years ago Closed 20 years ago

editor needs to detect attempt to load frameset document

Categories

(Core :: DOM: Editor, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: michael, Assigned: sfraser_bugs)

References

()

Details

Attachments

(2 files)

when openning an HTML page with a frameset tag, the editor freezes.
Target Milestone: M15
we can't create framesets, and for beta1 you won't be able to launch the editor
on an arbitrary document (via "edit page" or "file open" or ...).  So this bug
can wait until after beta entry.  In fact, I'm pretty sure it's a duplicate.
Severity: normal → critical
Status: NEW → ASSIGNED
Priority: P3 → P1
Target Milestone: M15 → M12
composer is in, so this is more important now.  marking it a critical P1
crasher.
Summary: Opening frameset page in editor causes Mozilla to freeze → [CRASH]Opening frameset page in editor causes Mozilla to freeze
using the 19991102 build, the editor doesn't freeze, it seems like the entire
desktop freezes. I had to select Ctrl+Alt+Delete and shut down the app to regain
control. This is quite a nasty little bug.
I have already started looking into this.  I have a partial fix (the app doesn't
lock up or crash), but it's not 100% correct yet, because the document does
still load.  We'd rather pop up a message saying "sorry, bub, you don't get to
edit frameset documents this year."  When M12 opens, I'll check in (at least)
the partial fix.
Summary: [CRASH]Opening frameset page in editor causes Mozilla to freeze → [CRASH][dogfood]Opening frameset page in editor causes Mozilla to freeze
crasher, not marked dogfood.   seems redundant to mark crashers as dogfood,
since crashing clearly prevents use!  but some crashers are worse than others.
The workaround is simply "don't open framesets", but the user doesn't always
know what is a frameset and what isn't.  So "edit page" can crash them
unexpectedly.
Summary: [CRASH][dogfood]Opening frameset page in editor causes Mozilla to freeze → [CRASH][BETA]Opening frameset page in editor causes Mozilla to freeze
Whiteboard: [PDT-]RN
Not needed for Dogfood...putting on PDT- radar....will Release Note feature
problem for dogfood.  Changed [DOGFOOD] in summary to [BETA]
Blocks: 18471
Severity: critical → normal
Priority: P1 → P3
Summary: [CRASH][BETA]Opening frameset page in editor causes Mozilla to freeze → [BETA]Opening frameset page in editor causes Mozilla to freeze
Whiteboard: [PDT-]RN
Target Milestone: M12 → M14
fixed the crash, but leaving bug open because we still don't do 100% the right
thing.
The frameset loads in the editor window, but no editor is actually attached, so
no editing is possible.  Next step is to detect the error case and stop the
document load altogether.
removed [CRASH} and [PDT-], lowering priority and severity and pushing out
milestone.
Summary: [BETA]Opening frameset page in editor causes Mozilla to freeze → Editor should disable the opening of frameset and xml documents
changed summary to reflect the partial fix I have in place.
cc-ing people who know about URL loading:  jud, travis, scott.

I need a way of find what the doctype is early in the loading process, so I can
abort the load and put up a dialog or whatever.  The point is, we don't want to
load a document we can't edit, and we're JUST and HTML editor for now.

If the editor sets itself up as an nsIDocumentLoaderObserver, is
OnStartDocumentLoad() the right place?  How do I ask the question "what is the
doctype of the doc you're about to load?"

Thanks
buster, by doc type do you mean the content type of the document?
yeah:  I need to differentiate between HTML and XML for starters.  I should be
able to differentiate between normal HTML and FRAMESET documents as well.
Assignee: buster → rpotts
Status: ASSIGNED → NEW
it sounds like you shouldn't even be envoked if you can't handle a doc. rather
than spitting it back if you can't handle it, the URI loader shouldn't hand it
to you in the first place. I'm assigning this to rpotts as he's spearheading the
URI targeting right now.
Oh great...  As if life wasn't sucky enough in URI targeting land - you had to
give this one to me :-)

I'm not sure if this is even a URI targeting issue at all - I'm sure you were
just waiting for me to say that!!

But, really, HTML doesn't have much in the way of structure...  So, the only
time you'll be *sure* that you don't have a FRAMESET document is *after* the
entire document has been parsed!  This is way past the URI targetting stage...

It seems like a better approach would be to register as a tag observer on the
parser and fail out of the load if a FRAMESET or XML document is encountered...

make any sense?
-- rick
Assignee: rpotts → buster
I'm kicking this one back to buster :-)  I don't think that URI targeting is
sufficient to fix the problem...

-- rick
marking potential beta1 bugs
Keywords: beta1
PDT+
Whiteboard: [PDT+]
assigning to rickg.  he needs to put in some infrastructure before the editor 
can do anything.
Assignee: buster → rickg
Steve -- Why does the parser need to do anything for an XML document? You 
already know the document type due to the actual document object that gets 
constructed. I'm proceeding on the basis that this is just a frameset issue.
Status: NEW → ASSIGNED
rick: what you said.  I know how to handle xml.  it's just framesets where I 
need parser/document support.
Buster: Another wonderment I have is why we want to put this interface on the 
contentsink, rather than use the tag observer mechanism that I already have. In 
essence, you would register as an observer of <frameset>, and I'll call you if I 
see it. You can return a status code to me to tell me to stop loading if that's 
the right answer (we already do that in charset detection). 

Doesn't that solve this issue for you?
After discussing with buster, I believe that the parsing engine already has all 
the functionality we need. Steve: get with me to learn how to use this feature; 
it's really, REALLY easy.
Assignee: rickg → buster
Status: ASSIGNED → NEW
Whiteboard: [PDT+] → [PDT+] 2/25
the parser tag observer works great for framesets.  but not at all for xml.
I'm going to split up this bug into several pieces:
1) editor needs to detect attempt to load frameset document (this bug)
2) editor needs to detect attempt to load xml document (bug 28972)
3) the editor UI needs to handle illegal document types (bug 28973)
4) the browser "Edit Page" action should be disabled if the current document 
loaded in the browser is a frameset or xml document (bug 28974)

PDT Team: I've nominated all of the derivative bugs, because it's not clear 
which part(s) of this are considered beta blockers.
Status: NEW → ASSIGNED
Summary: Editor should disable the opening of frameset and xml documents → editor needs to detect attempt to load frameset document
Whiteboard: [PDT+] 2/25 → [PDT+] fix in hand
in the patch I posted, travis suggests changing "aLoader" to "mWebShell" for the 
Stop() method.  Tested, and this works fine as well.
When do we expect to see the landing?  Please update the status whiteboard, or
even better... close the bug as fixed ;-)
Thanks :-)
Whiteboard: [PDT+] fix in hand → [PDT+] this will land 3/3 (because I'm travelling tomorrow)
since this code has no user-visible effect, and the bugs that depend on it are 
PDT-, I think this should be PDT- as well. Removed PDT+ for reconsideration.
Whiteboard: [PDT+] this will land 3/3 (because I'm travelling tomorrow)
Whiteboard: [PDT-]
PDT-
simon told me yesterday he was taking this one.  Working but partial fix is 
attached.  Three things still need to get done:
1) nsHTMLEditorParserObserver needs to support the weak reference API, so the 
parser can release it properly.  
FYI: RickG has a bug about how the parser releases these listeners which only 
effects listeners that register for more than one tag, so I think we're 
uneffected by that bug here since we only listen for <frameset>.
2) We need to decide what exactly to do in response to a frameset notification.  
Stop loading the document? Pop up a dialog? Bring up the editor window or kill 
it entirely? Charley has a bug on this part of it, I think
3) Different editor types would need to provide their own parser listeners. We 
need to decide how the shell gets the parser listener.  It should probably be 
dependent on the editor type, maybe even provided by the editor itself through 
some generic editor interface.  Except as I recall the parser listener is needed 
before the editor is actually instantiated, so maybe it's a static on the 
interface?

Like I said yesterday, sniffing for XML is an entirely different problem.  The 
parser observer is no help there.  mscott or rick potts can point you in the 
right direction for getting the "xml" out of the data stream from the doc 
loader, I hope.
Assignee: buster → sfraser
Status: ASSIGNED → NEW
move to M15 since this didn't make beta1 (PDT-)
Keywords: beta1
Whiteboard: [PDT-]
Target Milestone: M14 → M15
Some investigation as to why frameset documents continually reload in the editor.
The reload happens because, for some reason, nsHTMLFrameInnerFrame::Reflow() is 
called in such a way that it decides to call LoadURI to reload the frame 
documents over again. This only happens if we've made the 
PrepareDocumentForEditing() call in the editor. So something weird is happening 
in that call.
Status: NEW → ASSIGNED
It's the call to 
  styleSheets->ApplyOverrideStyleSheet("chrome://editor/content/
EditorOverride.css");
that is causing the trouble here. cc: charlie.
I checked some code in here, but it still needs work. The changes I checked in do 
two things:
1. If we let a frameset load in editor, the editor will not crash, but will end 
up sitting on the last document to load (which is actually editable).
2. There is code to detect a frameset tag, and an alert is shown if one is 
detected. We don't yet properly handle things from then on (since closing the 
window programmatically at that point crashes).
Fix checked in. We now show a dialog (just one) if you try to edit a frameset, 
then close the window.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
I just tried loading that page above using 4/4 build..I don't see any dialog
come up...instead I see the editor attempting to open that page with
frameset, the frame in the middle of the page keeps blinking...

reopening...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
ok I was uysing the wrong build.....but I tried it
again on 4/4 build and now we crash after clicking OK on that
framset panel.
This is actually fixed; we do detect the frameset now, and throw up the dialog 
OK. It crashes, but sujay is filing a separate bug on that.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
verified in 4/4 build.
Status: RESOLVED → VERIFIED
No longer blocks: 18471
You need to log in before you can comment on or make changes to this bug.