relative positioned elements flow around floats at new position instead of original

RESOLVED FIXED in mozilla1.7alpha

Status

()

Core
Layout: Floats
P1
normal
RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: Frank de Bot, Assigned: dbaron)

Tracking

({regression, testcase})

Trunk
mozilla1.7alpha
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch], URL)

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030507
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030507

The site
http://www.searchy.nl/traffic/mysqlstat/mysqlstat/cgi-bin/mysqlstat.cgi?t=s001.searchy.nl&skin=
was correctly generated in previous versions of mozilla (1.2.1), but no in the
current version 1.4beta.

Reproducible: Always

Steps to Reproduce:
1. start mozilla (duh)
2. Go to
http://www.searchy.nl/traffic/mysqlstat/mysqlstat/cgi-bin/mysqlstat.cgi?t=s001.searchy.nl&skin=
3. See it for your self

Actual Results:  
Shown website correctly

Expected Results:  
Show website on a wrong manner (It was correct in 1.2.1)

Updated

15 years ago
Summary: Site is wrongly shown in this mozilla. Previous versions the page was right → [searchy.nl] Site is wrongly shown in this mozilla. Previous versions the page was right

Comment 1

15 years ago
please attach a screenshot of how the website looked in 1.2.1 and how does it
look in 1.4beta. Thanx!!

Try shift + reload to see if that fixes the display.
(Reporter)

Comment 2

15 years ago
Created attachment 122904 [details]
Website in mozilla 1.2.1

Website in mozilla 1.2.1
(Reporter)

Comment 3

15 years ago
Created attachment 122905 [details]
Website in mozilla 1.4b

Website in mozilla 1.4b

Comment 4

15 years ago
Created attachment 122914 [details]
testcase

with Mozilla linux trunk 20030509, the green box with "bar"s is displayed to
the right of the red box with "foo"s.
with Mozilla 1.2.1, the green box is displayed below (and partially
overlapping) the red box.
(Assignee)

Comment 5

15 years ago
The space manager translation needs to ignore relative positioning.

This was probably exposed by making relative positioning not cause the creation
of a new space manager (float formatting context).
Assignee: table → float
Status: UNCONFIRMED → NEW
Component: Layout: Tables → Layout: Floats
Ever confirmed: true
QA Contact: madhur → ian

Updated

15 years ago
Keywords: regression, testcase
(Assignee)

Comment 6

15 years ago
Created attachment 122915 [details] [diff] [review]
the logic of what needs to be done to fix this

This patch contains the logic of what needs to be done to fix this. 
Unfortunately, it won't compile, and it's a bit of a pain to make it compile,
since we don't have a reflow state to get the computed offsets from during
state recovery.

This is the kind of bug that's hard to fix given our current architecture.
(Assignee)

Updated

15 years ago
OS: Linux → All
Hardware: PC → All
Summary: [searchy.nl] Site is wrongly shown in this mozilla. Previous versions the page was right → relative positioned elements flow around floats at new position instead of original
(Assignee)

Comment 7

15 years ago
*** Bug 194124 has been marked as a duplicate of this bug. ***

Comment 8

15 years ago
testcase attachment 123447 [details] from a duped bug 194124
Priority: -- → P3
(Assignee)

Updated

15 years ago
Priority: P3 → P1
(Reporter)

Comment 9

14 years ago
Created attachment 124827 [details]
Website in mozilla 1.4rc1

The website in mozilla 1.4rc1  The site shows different, but it still wrong.

Updated

14 years ago
Target Milestone: --- → Future
(Assignee)

Updated

14 years ago
Blocks: 218913
(Assignee)

Comment 10

14 years ago
I should just use a frame property to fix this, probably.
Target Milestone: Future → mozilla1.6alpha
(Assignee)

Updated

14 years ago
Blocks: 222739
(Assignee)

Comment 11

14 years ago
Created attachment 138313 [details] [diff] [review]
patch
Attachment #122915 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #138313 - Flags: superreview?(bz-vacation)
Attachment #138313 - Flags: review?(bz-vacation)
(Assignee)

Updated

14 years ago
Assignee: core.layout.floats → dbaron
Whiteboard: [patch]
Target Milestone: mozilla1.6alpha → mozilla1.7alpha
Comment on attachment 138313 [details] [diff] [review]
patch

>Index: layout/html/base/src/nsBlockReflowContext.cpp
>@@ -364,24 +373,41 @@ nsBlockReflowContext::ReflowBlock(const 
>+    if (offsets)
>+      *offsets = nsPoint(aComputedOffsets.left, aComputedOffsets.top);

Why not just |offsets->MoveTo(aComputedOffsets.left, aComputedOffsets.top) ? 
Doesn't really matter either way, I guess.

>Index: layout/html/base/src/nsBlockReflowState.cpp

>+    if (kid && !(kid->GetStateBits() & NS_BLOCK_SPACE_MGR)) {

Add a comment explaining why the NS_BLOCK_SPACE_MGR check needs to be there?

And add a comment here in general explaining why we're mucking with the coord
systems?

>+        nsPoint *offsets;
>+        nsIFrameManager *frameManager = mPresContext->GetFrameManager();
>+        frameManager->GetFrameProperty(kid,
>+                                       nsLayoutAtoms::computedOffsetProperty,
>+                                       0, (void**)&offsets);
>+        tx -= offsets->x;
>+        ty -= offsets->y;

If we had OOM earlier when allocating offsets, this is bad.  Should probably
null-check offsets....

>@@ -957,26 +970,24 @@ nsBlockReflowState::FlowAndPlaceFloat(ns

>-      nsIFrame* prevInFlow;
>-      floatFrame->GetPrevInFlow(&prevInFlow);

I assume that this was not supposed to be clobbering the prevInFlow from the
preceding code?  Or is it the same prevInFlow?

r+sr=bzbarsky with those comments addressed.
Attachment #138313 - Flags: superreview?(bz-vacation)
Attachment #138313 - Flags: superreview+
Attachment #138313 - Flags: review?(bz-vacation)
Attachment #138313 - Flags: review+
(Assignee)

Comment 13

14 years ago
Created attachment 138787 [details] [diff] [review]
patch

This addresses bz's comments.  (Regarding the last comment -- it's a fix for a
-Wshadow warning (I still use -Wshadow), and the existing variable was for the
same frame.)
Attachment #138313 - Attachment is obsolete: true
(Assignee)

Comment 14

14 years ago
Fix checked in to trunk, 2004-01-10 20:29 -0800.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.