680823 - The CSS resize property should work on iframes

Status: NEW

(Core :: Layout, defect)

Description

[(no longer active)]

It currently doesn't have any effect on iframes.

Neil, do you know why resize wouldn't apply on iframes?

Comment 1

[Neil Deakin]

Presumably, the resize property has to be passed onto the document loaded in the frame in the same way that the overflow property is. (in nsGfxScrollFrameInner::GetScrollbarStylesFromFrame() I would guess).

Comment 2

[(no longer active)]

Attachment: WIP 1

So, this WIP patch enables the resizer control to be displayed on the iframe.  But it can't resize the iframe using the mouse.  While I was trying to investigate why this happens, I came across this check: <http://mxr.mozilla.org/mozilla-central/source/layout/xul/base/src/nsResizerFrame.cpp#362> which is explicitly preventing this from working.  This check has been added in bug 489303.

Neil, roc: do you know if it's safe to disable this check?

Comment 3

[Neil Deakin]

Does the patch in bug 557049 help here?

Comment 4

[Neil Deakin]

Sorry, I meany bug 683394.

Comment 5

[(no longer active)]

(In reply to Neil Deakin from comment #4)
> Sorry, I meany bug 683394.

It does, in the sense that resizing the iframe does something.  But that thing is resizing the parent window, not the iframe itself.  :(

Comment 6

[(no longer active)]

Note to self: this patch causes this SVG test to crash by doing recursive restyles and running out of stack space: http://mxr.mozilla.org/mozilla-central/source/layout/svg/crashtests/587336-1.html?force=1

Similar bugs: bug 587336 and bug 601999.  Hoping that Daniel can tell me why...

Comment 7

[(no longer active)]

dholbert: ping?  (yes, I know that you're on vacation!  this ping is targeted at next week!)

Comment 8

[Daniel Holbert [:dholbert]]

I applied your patch (with fuzz to accommodate the recent bool switchover), and I was able to load 587336-1.html just fine.  (It loads immediately, with little-to-no CPU & memory usage. UI stays responsive, and I don't see any crashes after leaving it open for a minute or so.)

I did notice one odd thing though -- with your patch, that crashtest renders a "B" character.  Without the patch, it's blank.

Comment 9

[Daniel Holbert [:dholbert]]

(Sorry, disregard comment 8 --I'd disabled scripts in that source tree's test-profile, and had forgotten about it, so the crashtest's onload handler wasn't firing.  Now that I've re-enabled scripts, I do hit the problem - hang & crash-within-a-few-seconds.)

Comment 10

[Daniel Holbert [:dholbert]]

So, this is sort of the reverse of bug 601999.  There, we've got a parent who uses its child as a filter; in 587336-1.html, we've got a child that uses its parent as a filter.

Basically, what happens here is this:
 - Each reflow of the root scrollframe posts a reflow callback.
 - With current trunk, we service these callbacks asynchronously, so the UI remains responsive and we don't hit any recursion. (though we do get high CPU usage)
 - However, as described below, the patch here ends up making us set "shouldFlush" to *true* inside of HandlePostedReflowCallbacks, which makes us synchronously flush the new callback inside of HandlePostedReflowCallbacks, triggering a recursive death-spiral of nested HandlePostedReflowCallbacks / FlushPendingNotifications / ProcessReflowCommands / DidDoReflow calls.

Here's why "shouldFlush" ends up being set, with the attached patch:
 (1) This bug's patch tweaks nsHTMLScrollFrame::Reflow to make mInner.mCollapsedResizer default to *false*, instead of the current default of *true*.
 (2) As a result, 'reflowScrollCorner' ends up being true (instead of false as it otherwise would be)
 (3) As a result, we hit the clause that calls mInner.LayoutScrollbars()
 (4) At the end of LayoutScrollbars, we set mUpdateScrollbarAttributes to true.
 (5) As a result, when we reach nsGfxScrollFrameInner::ReflowFinished(), we actually complete that function and return PR_TRUE instead of taking the early PR_FALSE return (due to a "!mUpdateScrollbarAttributes" check).
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#3134
 (6) The boolean return-value from the previous step turns out to be all-important -- it tells the caller (HandlePostedReflowCallbacks) whether it needs to flush pending notifications ("shouldFlush") synchronously.
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#3859
 (7) As described at the beginning of this comment, we do have a callback posted -- we've got a reflow requrested for the scrolled frame's inner frame.  Since shouldFlush is now *true*, we evaluate that callback synchronously.
 (8) That callback triggers the code-path from (1) (nsHTMLScrollFrame::Reflow), so we end up recursively death-spiraling.

I'm not immediately sure which point the fix should come in, but this at least somewhat explains why this patch makes us fail. :)

ehsan: Are you sure that you want to change the default value of mCollapsedResizer (and by extension, the value of "reflowScrollCorner")?  If not, we could nip this in the bud up at (1)/(2) in my analysis above. :)  Otherwise, we may have to tweak the filter invalidation a bit... (I'm not sure where or under what conditions, though)

Comment 11

[(no longer active)]

This is way beyond my knowledge on this code!  Is there somebody else who knows this code better?  The change that I made to mCollapsedResizer makes sense with what I'm doing in the patch, but I'm not sure if these side-effects are expected, and how to fix them... :/

Comment 12

[Neil Deakin]

Ehsan, can you explain why you changed the setting of mCollapsedResizer from true to false? Root frames should only show resizers in <browser showresizer="true"/> The showresizer attribute can change and cause the scrollbar to reflow.

Comment 13

[(no longer active)]

(In reply to Neil Deakin from comment #12)
> Ehsan, can you explain why you changed the setting of mCollapsedResizer from
> true to false? Root frames should only show resizers in <browser
> showresizer="true"/> The showresizer attribute can change and cause the
> scrollbar to reflow.

But that changes when we want to make iframes resizable, right?  Please note that this patch was work in progress, and it is completely possible that I've basically missed how this code is supposed to work.  :-)

Comment 14

[Arkadiusz Michalski (Spirit)]

Unfortunately this problem still exists and limited usefulness for CSS resize property (eg. for all tools that return the results to the iframe). So posibility to change the size for this one element requires using another solution. In other browser we see that:
- Chrome works correctly
- IE11 doesn't support CSS resize property

Comment 15

[Florian Rivoal]

This is now clear in the CSS-UI specification that resizing iframes is allowed regardless of the value of the overflow property. There is a test the CSS-UI-3 test suite for this, passed by Chrome and Safari, but failed by Firefox:

https://test.csswg.org/harness/test/css-ui-3_dev/single/resize-012/format/html5/


Spec ref: https://drafts.csswg.org/css-ui-3/#resize

Comment 16

[Jonathan Watt [:jwatt]]

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)

Comment 17

[Valentin Hilbig]

There seems to be a workaround by wrapping iframe into a resizable div using flexbox:

.resizer { display:flex; margin:0; padding:0; resize:both; overflow:hidden }
.resizer > .resized { flex-grow:1; margin:0; padding:0; border:0 }

<div class="resizer">
<iframe class="resized" src="https://wikipedia.org/"></iframe>
</div>

So pure CSS can resize an iframe even in FF today. Perhaps it's time to look at this here again.

Comment 18

[walterpardo7]

Any chance we can upgrade the severity to this bug? It has been an eternity.