Closed Bug 631337 Opened 14 years ago Closed 14 years ago

cannot resize Firefox window on XP with Start Page (or any other page with position: relative in the body) loaded

Categories

(Core :: Layout, defect)

x86
All
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla2.0b12
Tracking Status
blocking2.0 --- final+

People

(Reporter: asa, Assigned: MatsPalmgren_bugz)

References

(Depends on 1 open bug, Regressed 1 open bug)

Details

(Whiteboard: [hardblocker])

Attachments

(3 files, 3 obsolete files)

When I have the sexy new Firefox Start Page loaded up, the resizer widget in the corner of the Window doesn't work. It works when I have other pages loaded. Something in Firefox Start (about:home) is breaking it. If this is confirmed, it should block. Testing 2011-02-02 build on Win XP.
OS: Mac OS X → Windows XP
The body element on that page has "position: relative". Removing that fixes this issue. Which means any page could cause this problem.
Using Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11, I can resize the page with about:home loaded. I am testing in a VM. I will say you have to be pretty precise about whether you place your mouse, but you can resize the page.
(In reply to comment #2) > Using Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11, I > can resize the page with about:home loaded. I am testing in a VM. I will say > you have to be pretty precise about whether you place your mouse, but you can > resize the page. Is this by clicking and dragging on the actual dotted resizer image or on the corner edge of the window frame?
Marcia is probably seeing what you and I see which is that the dotted triangle is not grabbable but the one pixel or two of Window frame is grabbable.
yes, I am able to grab the window frame and resize. The dotted triangle doesn't do anything for me on any page.
Attached file testcase (deleted) —
Display Lists when resizer is clicked: Background 0xb033db00(RootBox(window)(-1)) (0,0,66660,42540)(0,0,0,0) uniform Background 0xb033dbe8(DocElementBox(window)(-1)) (0,0,66660,42540)(0,0,0,0) opaque Clip (nil)() (0,0,66660,42540)(0,0,0,0) Background 0xaed56de0(Deck(deck)(18)) (0,0,66660,42540)(0,0,0,0) uniform Background 0xaed56ed8(Box(vbox)(0)) (0,0,66660,42540)(0,0,0,0) uniform Background 0xaea24ac8(Box(hbox)(1)) (0,7560,66660,34980)(0,0,0,0) uniform Background 0xaea24f10(Box(vbox)(3)) (0,7560,66660,34980)(0,0,0,0) uniform Background 0xaea38230(Box(tabbrowser)(0)) (0,7560,66660,34980)(0,0,0,0) uniform Background 0xaea38488(Box(tabbox)(-1)) (0,7560,66660,34980)(0,0,0,0) uniform Background 0xaea38a40(Deck(tabpanels)(0)) (0,7560,66660,34980)(0,0,0,0) opaque uniform Background 0xaea38cc8(XULScroll(notificationbox)(0)) (0,7560,66660,34980)(0,0,0,0) uniform Clip (nil)() (0,7560,66660,34980)(0,0,0,0) Background 0xaea38c58(Box(notificationbox)(0)) (0,7560,66660,34980)(0,0,0,0) uniform Clip (nil)() (0,7560,66660,34980)(0,0,0,0) Background 0xaea81130(Stack(stack)(0)) (0,7560,66660,34980)(0,0,0,0) uniform Clip (nil)() (0,0,66660,42540)(0,0,0,0) Clip (nil)() (0,7560,66660,34980)(0,0,0,0) WrapList 0xaea81470(FrameOuter(browser)(0)) (0,7560,66660,34980)(0,0,0,0) Background 0xaea81470(FrameOuter(browser)(0)) (0,7560,66660,34980)(0,0,0,0) uniform Clip 0xaea81470(FrameOuter(browser)(0)) (0,7560,66660,34980)(0,0,0,0) OwnLayer 0xae7806d0(Viewport(-1)) (0,7560,66660,34980)(0,0,0,0) Background 0xae780d50(HTMLScroll(html)(-1)) (0,7560,66660,34980)(0,0,0,0) uniform OwnLayer 0xacb0f078(ScrollbarFrame(scrollbar)(-1)) (0,0,0,0)(0,0,0,0) OwnLayer 0xacb2b1f0(ScrollbarFrame(scrollbar)(-1)) (0,0,0,0)(0,0,0,0) OwnLayer 0xad5ea300(Box(scrollcorner)(-1)) (0,0,0,0)(0,0,0,0) Clip (nil)() (0,7560,66660,34980)(0,0,0,0) CanvasBackground 0xae780bd0(Canvas(html)(-1)) (0,7560,66660,34980)(0,0,0,0) uniform Background 0xad5ea598(Block(html)(-1)) (0,7560,66660,34980)(0,0,0,0) uniform OwnLayer 0xad5ea078(Box(resizer)(-1)) (65760,41640,900,900)(0,0,0,0) WrapList 0xad5ea078(Box(resizer)(-1)) (65760,41640,900,900)(0,0,0,0) Background 0xad5ea078(Box(resizer)(-1)) (65760,41640,900,900)(0,0,0,0) Clip 0xad5ea958(Block(body)(1)) (0,7560,66660,34980)(0,0,0,0) WrapList 0xad5ea958(Block(body)(1)) (0,7560,66660,34980)(0,0,0,0) Background 0xad5ea958(Block(body)(1)) (0,7560,66660,34980)(0,0,0,0) uniform
The hit test returns the last frame. When position:relative is removed, the last three lines of the display list above are not present.
this looks like a more general graphics or layout issue. moving to layout and unassigning, doesn't look like something I can figure out with my layout knowledge.
Assignee: mak77 → nobody
Component: General → Layout
Product: Firefox → Core
QA Contact: general → layout
Summary: cannot resize Firefox window on XP with Start Page (about:home) loaded → cannot resize Firefox window on XP with Start Page (or any other page with position: relative in the body) loaded
::AppendToTop(aBuilder, aLists.Content(), scrollParts.PositionedDescendants(), mResizerBox, createLayersForScrollbars); Try appending to aLists.Outlines() instead?
Attached patch patch (obsolete) (deleted) — Splinter Review
Seems to work
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #510331 - Flags: review?(roc)
There is a risk that when resizeable elements are covered by other content, the resizer may be visible even when none of the rest of the element is. But we can't completely avoid that risk and keep the resizer on top of the inner contents at all times. For example, see this testcase: <div style="resize:both; overflow:hidden; width:100px; height:100px; border:1px solid black"> <div style="position:relative; top:50px; left:50px; z-index:10; width:100px; height:100px; background:cyan;"></div> </div> <div style="background:yellow; position:relative; top:-20px; height:100px; width:100px; background:yellow;"></div> So I think in general your patch here is what we want. But I think to minimize compatibility risks, we should NOT do this for resizers in textareas, since they have resizers by default and in that case there is no risk of the textarea content being rendered above the resizer, and we don't want textarea resizers to start popping above other content. The easiest way to do that is probably another flag in nsGfxScrollFrameInner.
Whiteboard: [hardblocker] → [hardblocker][has patch]
Whiteboard: [hardblocker][has patch] → [hardblocker]
Whiteboard: [hardblocker] → [hardblocker][has patch]
Whiteboard: [hardblocker][has patch] → [hardblocker]
We'll need some reftests here too.
(In reply to comment #9) > ::AppendToTop(aBuilder, aLists.Content(), > scrollParts.PositionedDescendants(), mResizerBox, > createLayersForScrollbars); > > Try appending to aLists.Outlines() instead? Hmm. I'm testing this change again and it doesn't seem to working any more. It's quite possible that I imagined that it was working before. Any other suggestions?
I don't understand how that could not work.
Attached patch make the resizer rel.pos. (obsolete) (deleted) — Splinter Review
The problem is that the rel.pos. <body> is already placed in the PositionedDescendants() list at that point, so adding the resizer to Content() or Outlines() isn't going to help. Maybe we can make the resizer rel.pos. with maximum z-index? It seems to work (also for the testcase in comment 11).
Comment on attachment 511073 [details] [diff] [review] make the resizer rel.pos. This patch seems to work ok for me. > resizer { > -moz-binding: url("chrome://global/content/bindings/resizer.xml#resizer"); >+ position: relative; >+ z-index: 2147483647; > } Would this change cause any problems for other resizers? Do we want to limit to only the window resizer?
Use the Content() list for replaced elements, and when the PositionedDescendants() is empty (as nothing can overlap the resizer in that case).
(In reply to comment #16) > Would this change cause any problems for other resizers? Not sure what kind of problem you're thinking of. > Do we want to limit to only the window resizer? It would be easy to do so if we want to minimize the risk.
Attached patch rel.pos. resizer, but only for the root frame (obsolete) (deleted) — Splinter Review
(In reply to comment #18) > Not sure what kind of problem you're thinking of. Don't know. Does making every resizer relative positioned with a high z-index have any side effects which change the layout in some way?
Attachment #511073 - Attachment is obsolete: true
Yes, the side-effect that roc noted in comment 11. If there is surrounding content that overlaps the resizable element the resizer will show through [if we place it on the PositionedDescendants() list]. That's why I added the extra bit of heuristics - exclude replaced elements and when PositionedDescendants() list of the scrolled frame is empty. It seems worth it to fix the case where a positioned child is below the resizer. We could improve it further by testing if the PositionedDescendants() list has any items that intersect the resizer... is there an easy way to do that?
I'm not asking about the GfxScrollFrame change, which should only affect painting, no? I'm asking about the change to xul.css and what effect it has.
+ PRBool pos = + mOuter->GetParent() == mOuter->PresContext()->PresShell()->GetRootFrame(); Use mIsRoot. Otherwise I think the approach in comment #19 is fine.
(In reply to comment #22) > I'm not asking about the GfxScrollFrame change, which should only affect > painting, no? Right, painting and hit testing for event targeting. > I'm asking about the change to xul.css and what effect it has. None, I hope. I checked the frame trees before and after and I saw no difference for the resizer. The computed style is different of course but XUL layout ignores all positioning AFAIK. For nsGfxScrollFrame, the resizer is placed explicitly at its position and I don't see anything that will break by the rel.pos. change. FWIW, the "attachment 511128 [details] [diff] [review]" patch passed unit tests on TryServer. I tried a few themes from AMO and they seemed to work fine with the patch. There is an existing bug with the resizer though: after resizing the window the first click after releasing the mouse button is ignored ("sticky effect"). This bug occurs also without my patch so I assume it's a known problem.
This is with roc's suggestion in comment 23.
Attachment #511139 - Attachment is obsolete: true
(In reply to comment #24) > There is an existing bug with the resizer though: after resizing the window the > first click after releasing the mouse button is ignored ("sticky effect"). > This bug occurs also without my patch so I assume it's a known problem. That might be bug 632953.
Assignee: enndeakin → matspal
Oh, I just filed bug 633169 because I couldn't find anything that matched. Yeah, it might be the same underlying problem.
Comment on attachment 511334 [details] [diff] [review] rel.pos. resizer, but only for the root frame, v2 I think the other patch is fine too, but this is lower risk.
Attachment #511334 - Flags: review?(roc)
Attachment #510331 - Attachment is obsolete: true
Whiteboard: [hardblocker] → [hardblocker][needs landing]
Whiteboard: [hardblocker][needs landing] → [hardblocker][needs landing] [has patch]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [hardblocker][needs landing] [has patch] → [hardblocker]
Target Milestone: --- → mozilla2.0b12
Status: RESOLVED → VERIFIED
OS: Windows XP → All
Verified on Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
Depends on: 733897
Regressions: 1648407
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: