Closed
Bug 14640
Opened 25 years ago
Closed 25 years ago
Composer can't cope with <iframe> tags
Categories
(Core :: DOM: Editor, defect, P1)
Core
DOM: Editor
Tracking
()
M14
People
(Reporter: marina, Assigned: ftang)
References
()
Details
(Keywords: testcase, Whiteboard: [TESTCASE])
Attachments
(1 file)
(deleted),
text/html
|
Details |
Those three International sites are crashing when attempting to open them in
Composer by going (File|Edit page...)
http://cnn.vg.no/
http://cnn.passagen.se/
http://www.cnndanmark.dk/
Steps to reproduce:
-open http://www.cnn.com;
-on the left pane you'll see5 International sites(Spanish,Portuguese, Swedish,
Norwedgien and Danish);
-go to the Swedish site;
-go File|Edit page:
//note: while redrawing it'll crash
(Spanish and Portug are OK)
Severity: major → critical
Status: NEW → ASSIGNED
Priority: P3 → P1
Target Milestone: M12
Summary: Apprunner crashes when attempting to open in Composer International sites → [blocker] Apprunner crashes when attempting to open in Composer International sites
Target Milestone: M12 → M11
thought about this a little more, and for the sake of stability it should be
M11.
The problem is we get 2 OnEndDocumentLoad notifications for this page.
We're prepared to handle that in nsEditorShell, except sometimes when we
get to this point we've already registered a private object as a selection
listener. Depending on the timing, the selection could still exist, and if it
does then it'll notify all it's listeners when it changes. But this listener
has an invalid reference to an already-destroyed editor instance. The code
below fixes the problem by unregistering the selection listener if it was ever
registered.
Here's the fix:
Index: nsEditorShell.cpp
===================================================================
RCS file: /cvsroot/mozilla/editor/base/nsEditorShell.cpp,v
retrieving revision 1.61
diff -r1.61 nsEditorShell.cpp
744a745,759
>
> // first, unregister the selection listener, if there was one
> if (mStateMaintainer)
> {
> nsCOMPtr<nsIDOMSelection> domSelection;
> // using a scoped result, because we don't really care if this fails
> nsresult result = GetEditorSelection(getter_AddRefs(domSelection));
> if (NS_SUCCEEDED(result) && domSelection)
> {
> domSelection->RemoveSelectionListener(mStateMaintainer);
> NS_IF_RELEASE(mStateMaintainer);
> mStateMaintainer = nsnull;
> }
> }
>
simon, please code review. do we need the same unregistering logic in the
editor shell's destructor?
chris, I think this fix should go into M11 if simon gives the green light.
simon will code review, but the root problem may actually be elsewhere. his
call whether this code is desirable to protect ourselves from fragility in other
modules, even if that's the case.
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 4•25 years ago
|
||
http://cnn.vg.no/ doesn't crash for me. http://www.cnndanmark.dk/ does (setting
URL)
Comment 5•25 years ago
|
||
Well, even with buster's change, this page continually reloads in the editor. I'm
getting endless OnEndDocumentLoad notifcations.
The page contains embedded IFRAMEs, animated GIFs, and all kinds of crap, so it's
hard to say what's going on here. I filed bug 14774 on this.
I do think we need to check in buster's diff, though. That at least fixes the
crash.
Updated•25 years ago
|
Summary: [blocker] Apprunner crashes when attempting to open in Composer International sites → Apprunner crashes when attempting to open in Composer International sites
Target Milestone: M11 → M12
Comment 6•25 years ago
|
||
Fix for crash checked in. there are still many problems trying to edit this page,
not least of which is that the page never stops reloading.
Updated•25 years ago
|
Summary: Apprunner crashes when attempting to open in Composer International sites → [CRASHER] Apprunner crashes when attempting to open in Composer International sites
Comment 7•25 years ago
|
||
what's the status of this bug -- is it still a crasher? THe fix was checked in
over a month ago -- please update.
Updated•25 years ago
|
Whiteboard: test case needed
Comment 8•25 years ago
|
||
This is still a problem -- the page continuously reloads in the editor, and
eventually crashes.
We need a reduced test case here.
Reporter | ||
Comment 10•25 years ago
|
||
adding teruko to the cc list. Teruko, do you have some test cases in Ender group
to test what's going on when this crasher page is loading in Composer?
Comment 11•25 years ago
|
||
I do not have test cases for this. I am investigating this.
Updated•25 years ago
|
Assignee: sfraser → ftang
Status: ASSIGNED → NEW
Comment 12•25 years ago
|
||
using build 1999111108, I can't reproduce the crash. I talked with buster and he
thought that ftang had fixed this already. I am reassigning to Frank -- Frank,
if you fixed this already, can you please mark as fixed -- thanks.
Assignee | ||
Updated•25 years ago
|
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•25 years ago
|
||
marina, please verify the status. Reopen if you still can reproduce it.
Updated•25 years ago
|
Status: RESOLVED → REOPENED
Comment 14•25 years ago
|
||
Frank, I tested this in 111708 M12 build. http://cnn.vg.no works fine.
However, when try to open as Composer http://cnn.passagen.se/ and
http://www.cnndanmark.dk/ Apprunner does not crash, but Apprunner tries to load
the page infinitely.
Updated•25 years ago
|
Resolution: FIXED → ---
Assignee | ||
Updated•25 years ago
|
Status: REOPENED → ASSIGNED
Comment 15•25 years ago
|
||
Does anyone have any idea why we are loading infinitely?
Has anyone been able to create a simple test case?
Need to determine how critical this is.
Updated•25 years ago
|
Summary: [CRASHER] Apprunner crashes when attempting to open in Composer International sites → Apprunner goes into infinite loop when attempting to open in Composer
Target Milestone: M12 → M13
Comment 16•25 years ago
|
||
moving to m13 and update summary to reflect the issue
Updated•25 years ago
|
Summary: Apprunner goes into infinite loop when attempting to open in Composer → Composer can't cope with <iframe> tags
Whiteboard: test case needed → [TESTCASE]
Comment 17•25 years ago
|
||
Test case below. Composer has a problem with iframes (whatever they are ;-). See
- you don't need to know much for the BugAThon. Anyway, if you have the snippet
<iframe src="http://any.url.you.like">
</iframe>
in a web page and Edit Page, it will infinitely reload the contents of the
iframe. The test case uses http://www.mozilla.org.
So now Mozilla comes with inbuilt DOS attacks ;-)
Hope this helps to fix it.
Gerv
Comment 18•25 years ago
|
||
Comment 19•25 years ago
|
||
I copied the test page to
http://blues/users/bobj/publish/test/iframe-14640.html
Running the 1/12/2000 build (ID 2000011210), it loads fine. No infinite loop.
Comment 20•25 years ago
|
||
I've just downloaded build 2000011210 (Win32) and, when I load up my local copy
of the attachment (I obviously can't get at your internal copy) it still
continuously refreshes when you go File | Edit Page and look at it in Composer
(although in M12 the title bar also changed; this is now fixed).
Are you absolutely sure you are not seeing this behaviour (as described in
sfraser@netscape.com's comment of 9/23) in Composer?
Do you see the problem on the original page?
Also, is this now a dupe of Bug 14774?
Gerv
Comment 21•25 years ago
|
||
Whoops. I did not read this carefully. I tried it in brower not composer.
Yes, in browser it loops infinitely, with the console window repeating:
The document was loaded in the content area
WEBSHELL+ = 17
WEBSHELL- = 18
...
Assignee | ||
Updated•25 years ago
|
Target Milestone: M13 → M14
Assignee | ||
Comment 22•25 years ago
|
||
move to M14 per bobj
Comment 23•25 years ago
|
||
Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!
Keywords: testcase
Assignee | ||
Comment 24•25 years ago
|
||
Change Plantform and OS to ALL
OS: Windows NT → All
Hardware: PC → All
Assignee | ||
Comment 25•25 years ago
|
||
mark this dup as 14774
*** This bug has been marked as a duplicate of 14774 ***
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•