437009 - buttons with type="radio" or type="checkbox" render with the incorrect state when checked

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Yuri Pimenov]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9) Gecko/2008051206 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9) Gecko/2008051206 Firefox/3.0

With classic look only button's label gets little offset when button is pressed. With winxp look nothing happens at all.

Reproducible: Always

Steps to Reproduce:
1. load sample xul in xulrunner/ff3.
2. try pressing buttons
3.



Same bug exists in XULRunner 1.9. Linux builds are unaffected.

Comment 1

[Yuri Pimenov]

dunno how to attach a file, posting it here.

<?xml version="1.0"?>
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>

<xul:window xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
  id="main"
  title="test"
  width="300"
  height="300"
  persist="width,height">

  <xul:hbox flex="1">
    <xul:vbox id="nav">
      <xul:button label="button1" type="radio" group="group1" />
      <xul:button label="button2" type="radio" group="group1" />
    </xul:vbox>
    <xul:caption flex="1" label="Hello World"/>
  </xul:hbox>

  <xul:separator class="groove-thin" />
  <xul:statusbar>
    <xul:spacer flex="1" />
    <xul:statusbarpanel label="Offline" />
  </xul:statusbar>

</xul:window>

Comment 2

[Gérard Talbot]

> dunno how to attach a file

Yuri, click "Add an attachment" link. Fill in the fields. Make sure you choose

o select from list: XUL (application/vnd.mozilla.xul+xml)

from the drop-down list.

Comment 3

[Yuri Pimenov]

Attachment: testcase

Comment 4

[Robert]

Confirm. Tested at my side and have reproducible-always as in description.

Comment 5

[Kai Liu]

Attachment: handle <button> checked/open states

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 327953 [details] [diff] [review]
handle <button> checked/open states

Hrm, wonder how we didn't catch this before.  I think this is ok -- I gues StandardGetState isn't enough, I thought the old code just did that amount

Comment 7

[Kai Liu]

(In reply to comment #6)
> Hrm, wonder how we didn't catch this before.
> 

Probably because nobody ever uses these button types.  ;)

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/index.cgi/mozilla-central/rev/1d847cc0dbf6

Comment 9

[Kai Liu]

Thanks, Dao, for driving the checkin-needed list!  :)

Comment 10

[Kai Liu]

Comment on attachment 327953 [details] [diff] [review]
handle <button> checked/open states

Requesting 1.9.0.2 approval: Very low-risk patch to make button states look consistent on Windows (this will also make the history sidebar's View button look right when open)

Comment 11

[Samuel Sidler (old account; do not CC)]

Can we get an automated test for this before landing on 1.9?

Comment 12

[Kai Liu]

Attachment: tests

I don't have a Mac to test these tests with, so that's why I'm excluding Macs from this test (this is, after all, a Windows bug...).  If someone could verify that these tests also work on the Mac, then the skip-if could be removed.

Comment 13

[Samuel Sidler (old account; do not CC)]

Comment on attachment 327953 [details] [diff] [review]
handle <button> checked/open states

Pushing approval out to 1.9.0.3.

Comment 14

[Samuel Sidler (old account; do not CC)]

Vlad can you review these tests?

Note, even with them, we're opting not to take this fix in a maintenance release.