387922 - Bug in nsGenericHTMLElement::GetOffsetRect() causing deviations in Position and Dimention calculations

Status: UNCONFIRMED

(Core :: DOM: Core & HTML, defect, P5)

Description

[Terje Rosenlund]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4

Many bugs related to position and dimention can probably be flaged as dublicates or caused by this bug

According to http://www.w3.org/TR/REC-CSS1#formatting-model - offsetTop, offsetLeft, offsetHeight and offsetWidth 
should not be affected by changes in style settings for margin, border or padding (offsetWidth and offsetHeight contains margin, border, padding and content)

This is not the case in Mozilla where offset-values are changed in 89 of 99 cases.
Not only are they changed but in 19 different ways where 12 rules are valid for only 1 elementName/Attribute combination

Mozilla also excludes DIV-tags from the offsetParent-chain and their layout-values are (partly) moved to the contained object
borderxxxWidth's are NOT moved, neather are scrollX and scrollY (would look silly too - a table width a scrollX...)
Info about scroll in DIV-tags is therefore lost from the chain
( https://bugzilla.mozilla.org/show_bug.cgi?id=343423 )

The purpose of the offsetParent-chain is to enable calculation of an objects position and dimention on the screen
A programmer needs 3 different position/dimention-set:

1. Outside  (offsetTop, offsetLeft, offsetHeight and offsetWidth
2. Inside	(Outside - margin, border, padding)
3. Visible  (Outside or inside container if container is smaller)

If the offset-values had not been mangled it would have been easy to write a 'one rule' function that calculate offsets for left, top, bottom, right, height and width 
As it is I need to map all blockElements to a 19-switch function to do the job

To make the mess complete - there is another bug which also inflicts on position / dimention calculations
( https://bugzilla.mozilla.org/show_bug.cgi?id=387328 )
The bug seems to be in the coor-function that applies classes to objects and causes all calculationes to be off by borderxxxWidth pix
The only solution at the time which eliminates this problem is to abandon style-sheets

As far as I know there is a long time before we will be blessed with the 3.0 version which hopefully is more precise in respect to these questiones
( and not written in the spirit of bug https://bugzilla.mozilla.org/show_bug.cgi?id=255754#c12 )

IE7 mangles only 31 of 87 cases and only by 5 different rules but has other issues (i.e. no padding for tables, inverted margin/border order in body, ...)

ps ! There is a -4px negative offset in the window-object on full blown pages (.. dont know wheter the value is used in internal calculationes or not)
     ( https://bugzilla.mozilla.org/show_bug.cgi?id=350290 )

Test app on: http://home.no.net/trosen/bugzilla/CheckPosDim/CheckPosDim.html

Reproducible: Always

Steps to Reproduce:
1.http://home.no.net/trosen/bugzilla/CheckPosDim/CheckPosDim.html
2.
3.

Comment 1

[Boris Zbarsky [:bzbarsky]]

offset* functions are not defined in any W3C spec; certainly not in CSS1.

They should basically do whatever is needed for reasonable compat with IE, modulo the bugs in IE's implementation of CSS that cause the actual box model in IE to differ from the one in Gecko.

Comment 2

[Terje Rosenlund]

(In reply to comment #1)
> offset* functions are not defined in any W3C spec; certainly not in CSS1.
 
http://www.w3.org/TR/REC-CSS1#formatting-model

CSS1 assumes a simple box-oriented formatting model where each formatted element results in one or more rectangular boxes. (Elements that have a 'display' value of 'none' are not formatted and will therefore not result in a box.) All boxes have a core content area with optional surrounding padding, border and margin areas.

 _______________________________________
|                                       |
|           margin (transparent)        |
|   _________________________________   |
|  |                                 |  |
|  |        border                   |  |
|  |   ___________________________   |  |
|  |  |                           |  |  |
|  |  |     padding               |  |  |
|  |  |   _____________________   |  |  |
|  |  |  |                     |  |  |  |
|  |  |  |  content            |  |  |  |
|  |  |  |_____________________|  |  |  |
|  |  |___________________________|  |  |
|  |_________________________________|  |
|_______________________________________|

          |    element width    |
|               box width               |

Comment 3

[Terje Rosenlund]

(In reply to comment #1)
> offset* functions are not defined in any W3C spec; certainly not in CSS1.

http://www.w3.org/TR/REC-CSS2/visuren.html#q1

Comment 4

[Phil Ringnalda (:philor)]

What are you hoping to point out with those links? Neither one of those specs, nor the CSS2.1 spec that's mostly what Gecko currently implements, includes the string "offsetT" which makes your contention that there's a spec compliance issue with the calculation of offsetTop (which Microsoft describes as "There is no public standard that applies to this property" and Mozilla's documentation describes as "DOM Level 0. Not part of specification.") rather unlikely. I'm willing to believe that there's one or more bugs, DOM or layout, contained somewhere within your report and/or testcase, but until you stop insisting that the W3C's CSS specs define a DOM property that isn't defined in any spec, it's going to be difficult to get to the actual meat of the matter.

Comment 5

[Terje Rosenlund]

(In reply to comment #4)
> What are you hoping to point out with those links? Neither one of those specs,
> nor the CSS2.1 spec that's mostly what Gecko currently implements, includes the
> string "offsetT" which makes your contention that there's a spec compliance
> issue with the calculation of offsetTop

The words 'offset*' is not used in the W3C spesification for CSS1 but ...

CSS1 box-oriented formatting model defines:

- a contained object is its containing box's content
- distance (offset) between content and anchor-point (top/left) is the containing box's padding + border + margin

Theese rules implies:

- Setting margin, border or padding on a containing box should only inflict on its content, not its own offset-values

Mozilla and IE7 are in conflict with this implicit rule

My test-app shows where - all values not Zero are wrong :)

Comment 6

[Terje Rosenlund]

I have looked into the source code and ended up in the basement

Seems like the poblem (.. or at least one of them) is located in nsGenericHTMLElement::GetOffsetRect(nsRect& aRect, nsIContent** aOffsetParent)
( /firefox/2.0.0.4/mozilla/content/html/content/src/nsGenericHTMLElement.cpp )

nsGenericHTMLElement::GetOffsetRect(nsRect& aRect, nsIContent** aOffsetParent)

636:
  // Get the Frame for our content
  nsIFrame* frame = nsnull;
  presShell->GetPrimaryFrameFor(this, &frame);

646:  
  nsIFrame* next = frame;				// TR: frame holds this frame
	
  do {
    rcFrame.UnionRect(rcFrame, next->GetRect());
    next = next->GetNextInFlow();
  } while (next);

// TR: All frames collected, start calculating position	
676:
	nsPoint origin(0, 0);

682:
    origin = frame->GetPosition();		// TR: frame = this, we are calculating positon for this, all zeros? (already set on 676:)

// .. still origin(0, 0);  (?)
699:
    parent = frame->GetParent();		// TR: First parent

    while (parent) {
	718:
		origin += parent->GetPosition();  // TR: Append parent offset
    }

/*
TR:
origin now holds the distance from nearest parent to origo
Still missing the distance from nearest parent to this upper/left corner AND adjusting this height and width 
(offset=margin+border+padding)

    parent = frame->GetParent();		// TR: Refetch First parent

	1: Append   parents left margin, border and padding to origin.x
	2: Append   parents top  margin, border and padding to origin.y
	3: Subtract parents left and right  margin, border and padding from rcFrame.width
	4: Subtract parents top  and bottom margin, border and padding from rcFrame.height

Both position and dimension ok by now, 765-795 should be deleted

*/

786-776 (add borders) are already disabled
As a side-effect the code below becomes a bug:
(which is causing ie. https://bugzilla.mozilla.org/show_bug.cgi?id=255754)

779:
  if (parent) {
786:
    if (includeBorder) {
      const nsStyleBorder* border = parent->GetStyleBorder();
      origin.x -= border->GetBorderWidth(NS_SIDE_LEFT);			// TR: Removing a border that was never appended
      origin.y -= border->GetBorderWidth(NS_SIDE_TOP);			// TR: Removing a border that was never appended
    }
  }
------------------------------------------------------
By the way: 

/firefox/2.0.0.4/mozilla/layout/xul/base/src/nsBoxObject.cpp, nsBoxObject::GetOffsetRect(nsRect& aRect)

has the same logic but without disabling add-borders which causes a inconstancy between objects created by nsGenericHTMLElement versus objects created by XUL

/firefox/2.0.0.4/mozilla/layout/xul/base/src/nsScrollBoxObject.cpp

nsScrollBoxObject is based on nsBoxObject

------------------------------------------------------
At last (for now):

It seems to be a bug in 
nsGenericHTMLElement::SetClassName(const nsAString& aClassName)
or
nsGenericHTMLElement::SetAttribute(const nsAString& aName, const nsAString& aValue)

which leaves borderXxxxWidth empty when border-style is applied by className

Comment 7

[Terje Rosenlund]

I have updated severity to critical and finds the lack of response puzzling, 
expecting bugs in core-functions to raise more attention.

My assertion about position and dimension has not been commented:

- Changing margin, border or padding on an element should not cause changes in the elements own offset or size, only on its content

If the assertion is true there is no doubt about the bug I have accounted for in nsGenericHTMLElement::GetOffsetRect 
and I guess there is lots of functions higher in the function-tree that are trying to compensate for this inaccuracy
(exceptions). 

Saw in another case that you where asking for a test-suite for testing out how browsers are handling offset-calculations. 
My test-suite are testing all DOM:Elements (111, all but APPLET which hangs and crash firefox if no source is given).

The test is done by saving position and dimension for an element with no offset-style before applying margin, border and padding one by one (removing previous).  
Position and dimension are compared to original and delta-values reported to screen
(should be all zeros if my assertion is true => for firefox the errorcount is 532)

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Please read the definition of a "critical" bug before changing the severity (you can see the definition by clicking the "Severity" link).

Comment 9

[Terje Rosenlund]

Hint taken :)

My test-app had a flaw - style applyed to BODY and HTML tags was not reset (fixed)
Error-count then increased by 1(?) to 533

Comment 10

[Terje Rosenlund]

> I'm willing to believe that there's one or more bugs, DOM or layout, contained
> somewhere within your report and/or testcase, but until you stop insisting that
> the W3C's CSS specs define a DOM property that isn't defined in any spec, it's
> going to be difficult to get to the actual meat of the matter.

I have never claimed that W3C's CSS specs defines this as a DOM property. 
My assertion is that there is something wrong with the calculations causing eg. mozilla to behave in a way that do not comply.

I have changed the summary for the case and hope the new one is more acceptable

Comment 11

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.

Comment 12

[Emili Humphrey]

Attachment: same problem

Thanks from mattbrazdoggmyfx1979.