Closed Bug 41878 Opened 24 years ago Closed 24 years ago

<html> anon. content of <checkbox> not repainted when 'value' attribute changes

Categories

(Core :: XBL, defect, P2)

Product:
Core
Component:
XBL
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: jrgmorrison, Assigned: eric)

References

(
URL
)

Details

(Whiteboard: [nsbeta2-][nsbeta3-] dirty reflow?)

Attachments

(1 file)

extended testcase; notice that RHS of <html> is sometimes truncated (not reflowed completely). [text/xul]
(deleted), text/xul
Details 
This bug was originally bug #41126, but that bug turned up a problem in 
JS_SetPrototype that is possibly unrelated to this original report. So, 
I am filing this bug to track the bug reported by Tony Robinson:

    calling setAttribute( 'value', "labelString") multiple times on a 
    checkbox appends the text to the label rather than replacing it like 
    it should.

Tony attached a testcase: 
  http://bugzilla.mozilla.org/showattachment.cgi?attach_id=9400
which demonstrated the problem clearly. 

However, that attachment now crashes in current builds : see 
   http://bugzilla.mozilla.org/show_bug.cgi?id=41876
Setting 'blocks'/'depends on' dependencies. 

Tony: any comments?
Blocks: 26609
Severity: normal → blocker
Depends on: 41876
Not sure what to say.  I actually didn't build the test case since I have code 
that demonstrates the problem just fine for me (see 26609) - not sure who did.   
Unless this gets fixed we can't import any address book files in text (tab and 
comma separated) format.  If someone can point me at the right section of code to 
start checking I'd be happy to track this down but I can't seem to easily find 
what gets executed when I call checkbox.setAttribute

Oops. I attached the testcase (I forgot). The new crash loading the testcase
is an unrelated parser or necko problem.

I assume part of the setAttribute code goes through nsXULElement.cpp; but I 
say 'part' because the value attribute is inherited back into the XBL binding 
for the checkbox, and I don't know where that happens.
Hmmmm, I think this should go to Hyatt and not to Ben.
Assignee: ben → hyatt
Nominating for nsbeta2.
Keywords: nsbeta2
Putting on [nsbeta2+] radar for beta2 fix. Will be minus on 6/22
Whiteboard: [nsbeta2+][6/22]
fixed.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Re-opening. The problem with having two child text nodes is fixed.

However, there is a remaining display problem. 

1) load (from disk) the testcase:
     http://bugzilla.mozilla.org/showattachment.cgi?attach_id=9400

2) note that the text "foo" is displayed

3) click the checkbox

4) note that the new value of the text 'foo1' is not visible. 

   (However, the 'xul:html' part of the XBL binding for checkbox now
    has a dotted border as part of the :focus style for 'xul:html')

5) hide/reveal the browser window: the text is now visible, as well as the 
   border.

6) you can repeat this. Each time the value is changed, the <html> part of 
   the XBL binding is not painted (or reflowed?).

(You can also get the text to show up by clicking elsewhere in the content
area to remove focus from the checkbox). 

Eric, since this is a layout|paint issue, could you have a look at this and 
see who this should go to?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This is Eric's.  It's not getting properly repainted.
Assignee: hyatt → evaughan
Status: REOPENED → NEW
Sorry Hyatt, AttributeChanged never gets called on the text object. I think 
there is some bug in XBL that sets the attribute but never notifies the text 
object with AttributeChanged. If you modify the example to be a text object 
AttributeChanged gets called and it works just fine:

<?xml version="1.0"?> 
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
<window xmlns:html="http://www.w3.org/TR/REC-html40"
        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
        xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
        orient="vertical"
        >
<text value="click the checkbox, then hide and reveal the browser window"/>

<text value="foo" id="asdf" onclick='doit();'/> 

<box flex="1"/>

<script>
var elt = document.getElementById('asdf');
var count = 0;
function doit() {
  var str = "foo" + count++;
  elt.setAttribute('value', str);
  var get = elt.getAttribute('value');
  dump(get + '\n');
}
</script>
</window>
Assignee: evaughan → hyatt
Cleaning up status whiteboard and marking beta2 minus (6/22 has passed)

This bug appears to have morphed significantly from the current title, and now 
involves some redisplay of changed text.  Could you update the summary?

It also sounds like this is basic, and will need to be fixed.  Assuming I'm 
groking the details, please nominate for nsbeta3.

Thanks,

Jim
Whiteboard: [nsbeta2+][6/22] → [nsbeta2-]
Changing summary "setAttribute for checkbox not working"

Before I nominate for nsbeta3, perhaps tonyr can comment on what functionality 
will be lost for the enduser (re: bug 26609), and whether that feature must
be in nsbeta2.
Summary: setAttribute for checkbox not working → <html> anon. content of <checkbox> not repainted when 'value' attribute changes
This prevents users from being able to import address books from a text format.  
I have no idea how important this is but for some users it may be the only way to 
easily populate their address book if they were not previously Netscape users.  
Especially Unix users.  For Mac & Win32 we support a few other common formats but 
text is always the fall-back since most any application can export to text.
Target Milestone: --- → M21
Target Milestone: M21 → M20
nominate for nsbeta3.  Comment from trudelle in bug 
http://bugzilla.mozilla.org/show_bug.cgi?id=44571 indicates that this bug may be 
the cause.  I'm nominating since this primarily affects mail and aim pref panels 
and the mail import tool.
Keywords: nsbeta3
Priority: P3 → P4
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3+]
Target Milestone: M20 → M18
Blocks: 44571
bumping priority to P3, since it is blocking a P3.
Priority: P4 → P3
Status: NEW → ASSIGNED
*** Bug 49623 has been marked as a duplicate of this bug. ***
Priority: P3 → P2
P2
Reassigning to evaughan.  Eric, don't be confused by your earlier comments in 
this bug... you put a breakpoint in nsTextBoxFrame.cpp that was never hit, 
and so you gave it back to me.

However checkboxes don't use <text>.  They use <html>... hence the painting 
problems.
Assignee: hyatt → evaughan
Status: ASSIGNED → NEW
Well I just tried John's original test case and it works fine. Is this 
reproducable anymore?
Status: NEW → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → WORKSFORME
It still happened a week or two ago (see dup).
Reopening, but removing [nsbeta3+]. I can't reproduce the painting problem 
fully, but there is a little bit of a painting problem left (see test case). 
Basically, if the painted region is wider than the previous painted region,
the right hand side of the text is not painted (i.e. the dimensions of the 
previous content are used as the "damaged region"). 

This though is now a minor defect in its current form. However, the other, 
related bug (bug 44571) is still active: the checkboxes in pref panels don't 
repaint on occasion.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-]
No longer depends on: 41876
nsbeta3-
Whiteboard: [nsbeta2-] → [nsbeta2-] [nsbeta3-]
trudelle, I needed this for bug 38433. I had to use a really bad workaround
(added a second row of <html>s right to the radio boxes, and aligned them with
"margin-top: 4px", IIRC). If this bug ships, the hack will have to ship, too.
Also, it is likely that I'm not the last one to run into this, and I needed a
full day to find the problem. Removing evaluation, so you can see this.
Whiteboard: [nsbeta2-] [nsbeta3-] → [nsbeta2-]
nsbeta3-, although evaughan may have fixed this as a side effect of another bug
he hasn't checked in yet.
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3-]
Target Milestone: M18 → Future
Whiteboard: [nsbeta2-][nsbeta3-] → [nsbeta2-][nsbeta3-] dirty reflow?
Fixed
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
verified fixed mac/linux/win32 20000903nn builds.
Status: RESOLVED → VERIFIED
This is still not working for me. I have tried the test case reported in bug 
49623 and wasn't able to change dynamically the value of a radio button. 
I can survive without it for B3 but we should fix it one day. Reopen!
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
I second ducarroz' observation.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Re-resolving this as fixed. The bug in bug 49623 may not be directly related to 
this, and this bug has already been redefined more than once. I will file a new
bug with a simple testcase.
verified fixed
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: