Download & Extend

One too many closing divs in checker settings tab

Project:Web Links
Version:6.x-1.4
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

Hi Nancy,
Just upgraded from 1.2 to 1.4 and have discovered that you have one extra closing div in the checker settings tab. I have traced it to the double closing divs in the very last form item $form['rescue']['weblinks_rescue_unpub'] where there should only be one. I guess it does not affect your theme very much, but it was very clear in my theme that something was wrong!

Tracking it down was made more interesting by the additional divs you have to show/hide sections depending on the values selected in the form. The corresponding closing divs in #suffix are often not in the same form item as their opening divs in #prefix. To help with this I have added comments within the generated code which will show up when you view the source in your browser. [this is even better if your browser does does colour coding of html source]. I have also added a \n in a couple of places where a div starting a new form item was enclosed in the suffix value, so that the generated code starts on a new line. All these things will help in future to identify if there is a div mis-match.

The attached patch was created against 1.4 but also applies cleanly to 1.x dev and is OK with 2.x-dev (accepting the few lines offset)

Hope this helps
Jonathan

AttachmentSize
_weblinks.admin_close_div.patch1_.txt1.81 KB

Comments

#1

Status:active» needs review

Well, duh, why didn't I see the right sidebar move to the bottom? I will fix this in 6.x-2.x and let Robert decide if it's worth porting to 1.x. I do wish the </div> would allow the class for documentation.

#2

Status:needs review» patch (to be ported)

#3

Status:patch (to be ported)» fixed

Committed to 6.x-1.x Marking the ticket fix. Will be in 6.x-1.5 when released.

Thanks
Robert

#4

Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

nobody click here