Constructor not called when hidden elt is unhidden in onload

RESOLVED DUPLICATE of bug 188496

Status

SeaMonkey
Preferences
RESOLVED DUPLICATE of bug 188496
17 years ago
13 years ago

People

(Reporter: Philip Nemec, Assigned: David Hyatt)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
Choosing the preference for images can get one of the radio button stuck on...

To reproduce - preferences / privacy & security / Images
select accept images from originating server...
select accept all

(accept images from originating server is still selected)

Comment 1

17 years ago
Yup, I see it on build 2001112803, Win98.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

17 years ago
-> morse
Assignee: sgehani → morse

Comment 3

17 years ago
Looks like a problem in the infrastructure so I'm reassigning to Hewitt.  I 
suspect that it's a regression because I'm sure this worked when I originally 
implemented it.

Below is a very simplified test case to demonstrate the problem.  Click on line 
1 or line 3 without clicking on line 2 and everything works fine.  But then 
click on line 2 and then go to line 1 or line 3.  The radio button on line 2 
remains on.

A related problem is that if you try to navigate from line to line using the 
arrow keys, you go from line 1 to line 3 and back to line 1, never landing on 
line 2.

I believe the problems are caused by the fact that line 2 is initially hidden 
and made visible when the onload handler fires.  So even though it is visible, 
the xul is not seeing it. 


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

<page xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
      onload="document.getElementById('line2').removeAttribute('hidden');">
  <radiogroup>
    <radio label="line 1"/>
    <radio label="line 2" id="line2" hidden="true"/>
    <radio label="line 3"/>
  </radiogroup>
</page>
Assignee: morse → hewitt
QA Contact: sairuh → tpreston

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.9

Comment 4

17 years ago
*** Bug 113976 has been marked as a duplicate of this bug. ***

Comment 5

17 years ago
*** Bug 117173 has been marked as a duplicate of this bug. ***

Comment 6

17 years ago
Raising severity from minor to normal.
*** Bug 118561 has been marked as a duplicate of this bug. ***
Linux too. OS->All
OS: Windows XP → All

Comment 9

17 years ago
I thought I raised the priority on this from minor to normal on 12-28-01, but it 
seems that I didn't.  Trying it again now.
Severity: minor → normal

Comment 10

17 years ago
Just wanted to mention I saw this on OpenVMS build 20020114 as well.

Comment 11

17 years ago
*** Bug 120915 has been marked as a duplicate of this bug. ***

Comment 12

17 years ago
won't have time for this soon, so giving to blake since he worked on radios recently
Assignee: hewitt → blaker
Status: ASSIGNED → NEW

Updated

17 years ago
Target Milestone: mozilla0.9.9 → ---

Comment 13

17 years ago
This is an embarrasing bug that IMO needs to be fixed for Mozilla 1.0.  It's 
less important for Mach V because image manager is not in the commercial 
product.

Please reconsider your removal of the 0.9.9 target milestone.
Keywords: nsbeta1

Comment 15

17 years ago
I attached a workaround patch for now.  After I check it in, I'll reassign this
to hyatt.  The radio's constructor is never getting called.  The workaround for
now is to default to visible and then display: none (which kills the frame)
image blocking isn't enabled.

Oh, I inverted the logic in my moz build just for testing but forgot to undo
that when I attached the patch. Fixed locally.

Comment 16

17 years ago
Created attachment 67621 [details] [diff] [review]
corrected patch
Attachment #67619 - Attachment is obsolete: true

Updated

17 years ago
Target Milestone: --- → mozilla0.9.9

Comment 17

17 years ago
morse, can you review the pref-images.xul patch please?

Comment 18

17 years ago
patch for pref-images.xul looks fine to me.  I see how the changing the 
default from hidden to not hidden would solve the problem.  So r=morse

However I don't understand what you accomplish by repeating the identical 
operation if you get an exception.  Why would the second attempt succeed if the 
first one didn't?  Perhaps a comment in the xul file would be in order here.

Comment 19

17 years ago
The exception would probably be caused by getPref if the pref doesn't exist.  
getElementById is unlikely to throw an error.  So if getting the pref fails, we 
default to hidden.  Unless you'd rather default to shown.

Comment 20

17 years ago
Oh, I read the patch too quickly and thought that it was the second statement in 
the try block that was failing.  That's why I didn't understand why you 
reexecuted that statement in the catch block.  Now I understand what you are 
doing.

To prevent others from getting confused, and to avoid duplicate code, the 
following might be a cleaner way to do it:

   var enabled = false;
   try {
     enabled = parent.hPrefWindow.getPref("bool", "imageblocker.enabled");
   }
   if (!enabled) {
     document.getElementById("haveImageBlocking").setAttribute("hidden", true");
   }    

My r=morse holds if you make the above change.

Comment 21

17 years ago
Cleaner yet might be

   var enabled;
   try {
     enabled = parent.hPrefWindow.getPref("bool", "imageblocker.enabled");
   } catch(ex) {
     enabled = false;
   }
   if (!enabled) {
     document.getElementById("haveImageBlocking").setAttribute("hidden", true");
   }    

Comment 22

17 years ago
*** Bug 123898 has been marked as a duplicate of this bug. ***

Comment 23

17 years ago
got sr=hewitt with suggested change, checked in. now reassigning to hyatt.
Assignee: blaker → hyatt
Summary: accept originating images radio gets stuck on → Constructor not called when hidden elt is unhidden in onload
Target Milestone: mozilla0.9.9 → ---
I wonder if this is same kinda of logic would fix the Account Wizard radio
button problem in bug 109997.
*** Bug 123897 has been marked as a duplicate of this bug. ***

Comment 26

17 years ago
nsbeta1+ per Nav triage team, assuming this is required for P3P
Keywords: nsbeta1 → nsbeta1+

Comment 27

17 years ago
I don't think this is needed for P3P...?  Even if there was some P3P UI that had
this problem, there's an easy workaround for this.
Keywords: nsbeta1+ → nsbeta1

Comment 28

17 years ago
Hmm. I thought Terri said she saw this in today's commercial build, and assumed
it must be P3P UI landing.  If it is just needed for Image Mgr, then it would
obviously not be needed for MachV.  Steve, could you please clarify why you
nominated it?

Comment 29

17 years ago
It's definitely not needed for p3p.  I nominated it because it was an 
embarrasement in the image-manager pref panel.  But Blake has since checked in a 
local fix for the image manager so there is no more embarassement.  Therefore 
there's no longer any embarrasement and I'll withdraw my nomination.
Keywords: nsbeta1
Is this still an issue?  If so, can someone attach a testcase?

Comment 31

14 years ago

*** This bug has been marked as a duplicate of 188496 ***
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → DUPLICATE
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.