287446 - [FIXr]default value on input not shown in an iframe if the iframe is position: relative

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Simon Jonsson]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; sv-SE; rv:1.7.6) Gecko/20050226 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; sv-SE; rv:1.7.6) Gecko/20050226 Firefox/1.0.1

This page: http://xonium.net/bugg2.html has an iframe. In the iframe there is an
input box with a value="buggybuggy".

This means the default text should be buggybuggy in that input box, but it's not.

No other things in the iframe have any affect on that what i think but!

The page that has the iframe has some things that makes the value="buggybuggy"
not visible.

The bugg.js and bugg.css that is linked to the document.

Bugg.js has some parts from the well known styleswitcher.js that many people use
to switch stylesheets. You can see it here http://xonium.net/styleswitcher.js

I removed all parts that seemed to not have anything to do with the bug.

bugg.css has the simple:

#id {
	position: relative;
}

It also works if i has it as a class with .id instead. Note! The <link> tag used
to import the css file needs a title.

The last part is that the iframe is inside the div tag with the id="id". The bug
doesn't work if i just has the css style in the html document like
style="position: relative;" and the div tag has no class or id.

I have also tested with different names on that style and the bug was stil there.

And i have tested to use a span instead of a div and the bug was still there, so
i assume it will be there in a p tag or anything else.

ONE more thing. Also check out this: http://xonium.net/bugg.html The bug is not
there!! The only difference is on this the iframe src is
http://xonium.net/buggiframe.html. On http://xonium.net/bugg2.html the iframe
src is http://xonium.1go.dk/buggiframe.html. Both pages buggiframe.html is
identical! So if you find no other explanation for this it is weird, it seems
like if the page is on another server the bug happens, if it's on the same
server it doesn't? Ok i haven't tested on other servers yet but i will.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.


Expected Results:  
I should be able to see the value. I see it it internet explorer for example. ok
i know IE always corrects wrong code and makes it look good anyway, but is it
anything wrong here?

Comment 1

[José Jeria]

Also no text in input field with todays trunk build.

Comment 2

[Jason Barnabe (np)]

Attachment: testcase

The testcase requires that the page in the iframe be loaded remotely.

Comment 3

[Simon Jonsson]

Attachment: Testcases, float: left etc.

Some more testcases, with float: left, float: right, position: absolute  -  the
input box values isn't shown either.

Comment 4

[Boris Zbarsky [:bzbarsky]]

This is basically the same as bug 287120

Comment 5

[Boris Zbarsky [:bzbarsky]]

Or perhaps not...

The real problem with this testcase is that the main page is on one server and
the iframe is on a different one.  Which means they have different security
contexts.

When we kill off the subframe's presshell (because style changes cause a layout
frame reconstruct in the parent document), the text control tries to stash its
current value in its DOM node.  The code to stash away the value serializes the
editor's contents into a string, and then sets the value to that string. 
Serializing creates a DOM range on the relevant nodes in the editor and
serializes the range.

The problem is that creation of the range uses the DOM range methods.  In this
case that means that we do security checks in them.  The subject principal is
the JS that caused the style change (in the parent document).  The object
principal is that of the anonymous nodes in the child (so the principal of the
child document).  The principals don't match, so we get a security exception
(which editor ignores).

So I see the following options:

1)  The text control frame should escalate its privileges in some cases before
    calling into the editor.
2)  The editor should in some cases use non-DOM apis which don't do security
    checks to init its ranges.

We've had issues with this before (see bug 219848 comment 24) but at the time it
really _was_ JS accessing the data, just JS with expanded privileges.  Now we
just need the data for our internal use....

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 180732 [details] [diff] [review]
Proposed fix

- In nsTextControlFrame::SetValue():

+      // Time to mess with our security context... See comments in GetValue()
+      // for why this is needed.  Note that we have to do this up here,
because
+      // otherwise SelectAll() will fail.
+      nsCOMPtr<nsIJSContextStack> stack =
+	 do_GetService("@mozilla.org/js/xpc/ContextStack;1");
+      PRBool pushed = stack && NS_SUCCEEDED(stack->Push(nsnull));
+
+      nsCOMPtr<nsISelection> domSel;
+      nsCOMPtr<nsISelectionPrivate> selPriv;
+      mSelCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
getter_AddRefs(domSel));
+      if (domSel)
+      {
+	 selPriv = do_QueryInterface(domSel);
+	 if (selPriv)
+	   selPriv->StartBatchChanges();
+      }
+
       mSelCon->SelectAll();
       nsCOMPtr<nsIPlaintextEditor> htmlEditor = do_QueryInterface(mEditor);
       if (!htmlEditor) return;

Early return after we've pushed a null JS context onto the stack. That's not
good. Either move code around here, or pop if we return early here.

r+sr=jst with that fixed.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: With that fixed

Requesting 1.8b2 approval.  This patch makes us properly save and restore form
state in subframes that come from a different host.  It should be reasonably
safe, I think....

Comment 9

[chris hofmann]

Comment on attachment 181837 [details] [diff] [review]
With that fixed

a=chofmann

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 11

[Jason Barnabe (np)]

Verified. Simon, the fix will be in 1.1.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Added a test for this as part of bug 477700.