The CSS resize property should work on iframes

NEW
Unassigned

Status

()

defect
8 years ago
3 months ago

People

(Reporter: Ehsan, Unassigned)

Tracking

({dev-doc-needed, parity-chrome})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
It currently doesn't have any effect on iframes.

Neil, do you know why resize wouldn't apply on iframes?
OS: Mac OS X → All

Comment 1

8 years ago
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).
(Reporter)

Comment 2

8 years ago
Posted patch WIP 1Splinter Review
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?
(Reporter)

Updated

8 years ago
Keywords: dev-doc-needed

Comment 3

8 years ago
Does the patch in bug 557049 help here?

Comment 4

8 years ago
Sorry, I meany bug 683394.
(Reporter)

Comment 5

8 years ago
(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.  :(
Assignee: nobody → ehsan
Depends on: 683394
(Reporter)

Comment 6

8 years ago
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...
(Reporter)

Comment 7

8 years ago
dholbert: ping?  (yes, I know that you're on vacation!  this ping is targeted at next week!)
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.
(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.)
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)
(Reporter)

Comment 11

8 years ago
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

8 years ago
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.
(Reporter)

Comment 13

8 years ago
(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.  :-)
(Reporter)

Updated

7 years ago
Assignee: ehsan → nobody
Hardware: x86 → All
Whiteboard: [parity-chrome]
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

2 years ago
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
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Keywords: parity-chrome
Whiteboard: [parity-chrome]
You need to log in before you can comment on or make changes to this bug.