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)
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)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•14 years ago
|
OS: Mac OS X → Windows XP
Comment 1•14 years ago
|
||
The body element on that page has "position: relative". Removing that fixes this issue. Which means any page could cause this problem.
Comment 2•14 years ago
|
||
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.
Comment 3•14 years ago
|
||
(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?
Reporter | ||
Comment 4•14 years ago
|
||
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.
Comment 5•14 years ago
|
||
yes, I am able to grab the window frame and resize. The dotted triangle doesn't do anything for me on any page.
Comment 6•14 years ago
|
||
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
Comment 7•14 years ago
|
||
The hit test returns the last frame.
When position:relative is removed, the last three lines of the display list above are not present.
Comment 8•14 years ago
|
||
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
Updated•14 years ago
|
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?
blocking2.0: ? → final+
Comment 10•14 years ago
|
||
Seems to work
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]
Reporter | ||
Updated•14 years ago
|
Whiteboard: [hardblocker] → [hardblocker][has patch]
Whiteboard: [hardblocker][has patch] → [hardblocker]
Reporter | ||
Updated•14 years ago
|
Whiteboard: [hardblocker] → [hardblocker][has patch]
Whiteboard: [hardblocker][has patch] → [hardblocker]
Attachment #510331 -
Flags: review?(roc) → review-
We'll need some reftests here too.
Comment 13•14 years ago
|
||
(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.
Assignee | ||
Comment 15•14 years ago
|
||
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 16•14 years ago
|
||
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?
Assignee | ||
Comment 17•14 years ago
|
||
Use the Content() list for replaced elements, and when the
PositionedDescendants() is empty (as nothing can overlap
the resizer in that case).
Assignee | ||
Comment 18•14 years ago
|
||
(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.
Assignee | ||
Comment 19•14 years ago
|
||
Comment 20•14 years ago
|
||
(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?
Assignee | ||
Updated•14 years ago
|
Attachment #511073 -
Attachment is obsolete: true
Assignee | ||
Comment 21•14 years ago
|
||
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?
Comment 22•14 years ago
|
||
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.
Assignee | ||
Comment 24•14 years ago
|
||
(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.
Assignee | ||
Comment 25•14 years ago
|
||
This is with roc's suggestion in comment 23.
Attachment #511139 -
Attachment is obsolete: true
Comment 26•14 years ago
|
||
(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
Assignee | ||
Comment 27•14 years ago
|
||
Oh, I just filed bug 633169 because I couldn't find anything that matched.
Yeah, it might be the same underlying problem.
Assignee | ||
Comment 28•14 years ago
|
||
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)
Assignee | ||
Updated•14 years ago
|
Attachment #510331 -
Attachment is obsolete: true
Attachment #511334 -
Flags: review?(roc) → review+
Whiteboard: [hardblocker] → [hardblocker][needs landing]
Reporter | ||
Updated•14 years ago
|
Whiteboard: [hardblocker][needs landing] → [hardblocker][needs landing] [has patch]
Assignee | ||
Comment 29•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [hardblocker][needs landing] [has patch] → [hardblocker]
Target Milestone: --- → mozilla2.0b12
Updated•14 years ago
|
Status: RESOLVED → VERIFIED
OS: Windows XP → All
Comment 30•14 years ago
|
||
Verified on Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
You need to log in
before you can comment on or make changes to this bug.
Description
•