Legend change its position while changing value of a select box in the same fieldset

RESOLVED FIXED in mozilla1.9.1a1

Status

()

--
minor
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: noguer, Assigned: mats)

Tracking

({regression, testcase, verified1.9.0.2})

Trunk
mozilla1.9.1a1
regression, testcase, verified1.9.0.2
Points:
---
Bug Flags:
wanted1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ca; rv:1.9pre) Gecko/2008040907 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; ca; rv:1.9pre) Gecko/2008040907 Minefield/3.0pre

I have this simple HTML, every time I select a new value in the selectbox, the legend of the fieldset change its position.

But If I delete "top:110px", or  "bottom:0px" or "margin-left:-50px" it works fine.

<html><head>
	<style>
.cContingut {	position:fixed;	top:110px;	bottom:0px;}
.cContingut fieldset { padding-left:65px; }
.cContingut legend  { margin-left: -50px; }
</style>
</head> 
<body>
<div class="cContingut">
<fieldset>
<legend>2 &nbsp;&nbsp;	Esculli el compte vinculat</legend>
<select id="Compte" name="Compte" size="1">	
<option >a</option>
<option >a</option>
<option >a</option>
<option >a</option>
<option >a</option>
</select>
</fieldset>
</body></html>

Reproducible: Always

Steps to Reproduce:
1. Load the page in Firefox 3, in Firefox 2 it works fine.
2. Select a value in the selectbox

Actual Results:  
The lengend change its position

Expected Results:  
The legend should rest in the same position all the time
(Assignee)

Comment 1

11 years ago
Created attachment 320952 [details]
Reporter's testcase
(Assignee)

Updated

11 years ago
Severity: normal → minor
Status: UNCONFIRMED → NEW
Component: General → Layout: Form Controls
Ever confirmed: true
Keywords: regression, testcase
OS: Windows XP → All
Product: Firefox → Core
QA Contact: general → layout.form-controls
Hardware: PC → All
Version: unspecified → Trunk
(Assignee)

Updated

11 years ago
Assignee: nobody → mats.palmgren
(Assignee)

Comment 2

11 years ago
The problem is that we never get the real value for 'legendMargin'
if we don't reflow the legend frame (we will use a zero margin),
this makes us think we need to place the legend somewhere else:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/forms/nsFieldSetFrame.cpp&rev=3.168&root=/cvsroot&mark=446,448,450,553#444
(Assignee)

Comment 3

11 years ago
Created attachment 320956 [details] [diff] [review]
Patch rev. 1

The fix is the last change.  The rest are just some style nits.
Attachment #320956 - Flags: superreview?(dbaron)
Attachment #320956 - Flags: review?(dbaron)
(Assignee)

Comment 4

11 years ago
Created attachment 320957 [details] [diff] [review]
reftest.diff
Comment on attachment 320956 [details] [diff] [review]
Patch rev. 1

r+sr=dbaron, except I think it would make more sense to put your new code as an additional else to the already-existing chain:

if (reflowLegend) {
  ...
} else if (!mLegendFrame) {
  ...
} else {
  // your new line to set legendMargin here
}

Any reason that wouldn't work?
Attachment #320956 - Flags: superreview?(dbaron)
Attachment #320956 - Flags: superreview+
Attachment #320956 - Flags: review?(dbaron)
Attachment #320956 - Flags: review+
Does that work for percentage margins?  I'd think this should use GetUsedMargin(), not GetStyleMargin()->GetMargin().

In any case, we need a reftest here that also tests percentage margins.
er, yeah, this should use GetUsedMargin.
(Assignee)

Comment 8

11 years ago
Created attachment 320968 [details]
Testcase #2

Yes, GetUsedMargin() was what I meant.  Actually, percentage margins isn't
working at all for legends.  I think the first call should also be using
GetUsedMargin() and moved after the reflow...  I'll fix that too...
Attachment #320956 - Attachment is obsolete: true
Attachment #320957 - Attachment is obsolete: true
(Assignee)

Comment 9

11 years ago
Created attachment 320998 [details]
Testcase #3
Attachment #320968 - Attachment is obsolete: true
(Assignee)

Comment 10

11 years ago
Created attachment 321001 [details] [diff] [review]
Patch rev. 2

Use GetUsedMargin() in both places.
Attachment #321001 - Flags: superreview?(dbaron)
Attachment #321001 - Flags: review?(dbaron)
(Assignee)

Comment 11

11 years ago
Created attachment 321002 [details] [diff] [review]
reftest2.diff
Comment on attachment 321001 [details] [diff] [review]
Patch rev. 2

r+sr=dbaron assuming that you've tested that the % margins are %s of something sensible (i.e., the containing block width, for some reasonable definition of containing block)
Attachment #321001 - Flags: superreview?(dbaron)
Attachment #321001 - Flags: superreview+
Attachment #321001 - Flags: review?(dbaron)
Attachment #321001 - Flags: review+
(Reporter)

Comment 13

10 years ago
Is there any solution for this ? The problem is the same in the last Firefox 3  -  Mozilla/5.0 (Windows; U; Windows NT 5.1; ca; rv:1.9pre) Gecko/2008060306 Minefield/3.0pre)  
Er... so how come we never checked this fix in?  :(
Flags: wanted1.9.1?
Flags: wanted1.9.0.x?
(Assignee)

Comment 15

10 years ago
For RC2 you mean?  This wouldn't have been approved, so I didn't ask.
(I will land it on mozilla-central soon)
Ah, were we up to rc2 by then?  Yeah, this wouldn't have gone in at that late stage.

I still think this is worth fixing in 1.9.0.x.
Keywords: checkin-needed
Could you check this in?  And maybe add a testcase for comment 12, and double-check that it's sensible?
(Assignee)

Comment 18

10 years ago
Both Linux unit test Tinderboxes are orange and I've been waiting HOURS for
them to go green...  The reftest covers % margins.
Well, I've added checkin-needed, so hopefully somebody else can land it for you at the next rare opportunity.
(Assignee)

Comment 20

10 years ago
http://hg.mozilla.org/mozilla-central/index.cgi/rev/ba694e7645ea
http://hg.mozilla.org/mozilla-central/index.cgi/rev/69e0da0cdb2f

The reftest failed on qm-win2k3-03 for some reason so I disabled
it temporarily while I investigate:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/b863a7474393

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: wanted1.9.1? → in-testsuite?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a1
(Assignee)

Comment 21

10 years ago
Fixed and re-enabled the reftest:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/b41ed6fdb780
Flags: in-testsuite? → in-testsuite+
(Assignee)

Updated

10 years ago
Attachment #321001 - Flags: approval1.9.0.2?
Comment on attachment 321001 [details] [diff] [review]
Patch rev. 2

Approved for 1.9.0.2. Please land in CVS. a=ss

Please be sure to land the tests in CVS as well.
Attachment #321001 - Flags: approval1.9.0.2? → approval1.9.0.2+
(Assignee)

Comment 23

10 years ago
Landed on CVS trunk for 1.9.0.2:
mozilla/layout/forms/nsFieldSetFrame.cpp 	3.169
mozilla/layout/reftests/bugs/433700-ref.html 	1.1
mozilla/layout/reftests/bugs/433700.html 	1.1
mozilla/layout/reftests/bugs/reftest.list 	1.457 
Flags: wanted1.9.0.x?
Keywords: fixed1.9.0.2

Comment 24

10 years ago
Created attachment 335384 [details]
testcase: legend with margin-top moves when browser resized

A repro of a related problem that may or may not have the same cause as the existing bug.  Legend moves when browser (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1) is resized.

Comment 25

10 years ago
I hit a similar bug where the legend (set with position: relative and a
negative margin) of a fieldset moves. I see it when the browser is resized
rather than when a select element is used.  Can someone tell me if this is the
same bug and if the same fix will apply or if this needs to be entered as a new
bug?  Is the fix for this bug in a release version yet?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208
Firefox/3.0.1

See attachment LegendMoveRepro.html "testcase: legend with margin-top moves
when browser resized."
Verified for 1.9.0.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2) Gecko/2008090212 Firefox/3.0.2.
Keywords: fixed1.9.0.2 → verified1.9.0.2
You need to log in before you can comment on or make changes to this bug.