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

VERIFIED FIXED in mozilla2.0b12

Status

()

defect
--
major
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: asa, Assigned: mats)

Tracking

(Depends on 1 bug)

Trunk
mozilla2.0b12
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [hardblocker])

Attachments

(3 attachments, 3 obsolete attachments)

Reporter

Description

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

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

Comment 4

8 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.
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

8 years ago
Posted file testcase
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

8 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.
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?

Comment 10

8 years ago
Posted patch patch (obsolete) — 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.
Reporter

Updated

8 years ago
Whiteboard: [hardblocker] → [hardblocker][has patch]
Whiteboard: [hardblocker][has patch] → [hardblocker]
Reporter

Updated

8 years ago
Whiteboard: [hardblocker] → [hardblocker][has patch]
Whiteboard: [hardblocker][has patch] → [hardblocker]
We'll need some reftests here too.

Comment 13

8 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

8 years ago
Posted patch make the resizer rel.pos. (obsolete) — 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 16

8 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

8 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

8 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.

Comment 20

8 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

8 years ago
Attachment #511073 - Attachment is obsolete: true
Assignee

Comment 21

8 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

8 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

8 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

8 years ago
This is with roc's suggestion in comment 23.
Attachment #511139 - Attachment is obsolete: true

Comment 26

8 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

8 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

8 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

8 years ago
Attachment #510331 - Attachment is obsolete: true
Whiteboard: [hardblocker] → [hardblocker][needs landing]
Reporter

Updated

8 years ago
Whiteboard: [hardblocker][needs landing] → [hardblocker][needs landing] [has patch]
Assignee

Comment 29

8 years ago
http://hg.mozilla.org/mozilla-central/rev/a28d7d3ce0f0
Status: ASSIGNED → RESOLVED
Last Resolved: 8 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

Updated

7 years ago
Depends on: 733897
You need to log in before you can comment on or make changes to this bug.