WildcardSearch / Advanced-Sidebox

A plugin for MyBB forums that displays custom boxes on various forum pages.
GNU General Public License v3.0
20 stars 10 forks source link

multiple issues caused by closing tags of sidebox inserted in wrong position. #3

Closed avril-gh closed 11 years ago

avril-gh commented 11 years ago

Test environment - clean MyBB 1.6.9 / Adv Sidebox 1.3 / PHP 5.4.10

Issue's :

  1. Certain sideboxes (eg. search sidebox) not appear on expected position. Instead they show self above or below forums.
  2. Collapse function does not work for certain sideboxes (eg. search sidebox)
  3. Some other sideboxes (eg. custom sidebox) act as working ok (may collapse and can be placed on left or right), however the bug is visible in html output plus footer still is not positioned as expected. (100% width)
  4. When sideboxes are published, footer of MyBB is not positioned as expected (sideboxes columns take space on its sides while they shouldnt)
  5. W3C html validator shows that layout are broken.

Cause : All mentioned above issues are caused by inserting 'sideboxes closing code' not only at wrong position (after footer while it should be inserted before footer) but also with 2 extra closing div tags which finally break layout. By 'sideboxes closing code' (which is inserted after footer) i understand this part :

    </td></tr></table></div><div>

Which then create html output like this :

                    </div>
                    </div> 
                    <!-- end: footer -->
                    </td>
                </tr>
            </table>
        </div>
    </div>

These 2 divs at top are contained within footer itself and after adding sideboxes code below footer there are allready 4 closing divs total, which are (2 extra closing divs) breaking layout. Whats more, inserting it after footer cause sideboxes push footer and create blank space (columns ) on its sides.

Fix : 'sideboxes closing code' should be inserted before footer. Also assuming that footer allready contain these 2 required closing divs, we should insert only 'closing tables tags' without closing divs.

    </td></tr></table>

Which then would give :

                        <!-- (...) -->

                        </td>
                    </tr>
                </table>

                <!-- (...) -->
                </div>
                </div> 
                <!-- end: footer -->

Proof of concept : There are more places in code where 'sideboxes closing code' is inserted which should be fixed, but for quick demonstration i changed it in one place only.

Tested with one 'custom sidebox' published on left side. file 'adv_sidebox.php line 235 original code : (inserts stuff with extra divs after footer)

$templates->cache['index'] = str_replace('{$footer}', '{$footer}</td></tr></table></div></div>', $templates->cache['index']);

file 'adv_sidebox.php line 235 fixed code : (insert stuff before footer without divs assuming that required divs are allready included within footer)

$templates->cache['index'] = str_replace('{$footer}', '</td></tr></table>{$footer}', $templates->cache['index']);

After fixing this, html output appear to be valid (tested with W3C html validator), also footer maintain its 100% width and sidebox column are in expected position.

WildcardSearch commented 11 years ago

Thank you so much for your efforts to test this plugin. I will get it straightened out in 24 hours or less.

avril-gh commented 11 years ago

Thank you. The plugin is wonderful. It will be for sure one of most downloaded plugins for MyBB

WildcardSearch commented 11 years ago

I tested those changes on localhost and it is much better imo. Why didn't I think of that (or Nayar)?

Thanks and let me know if that solves the bugs and invalid HTML.

avril-gh commented 11 years ago

let me know if that solves the bugs and invalid HTML.

Checked it and it works perfectly perfect now. No problems with collapsing, sideboxes columns are in proper positions, footer is as it should be and no invalid html.

(however there is one small typo in random quote module) in file inc/plugins/adv_sidebox/modules/rand_quote/adv_sidebox_module.php line 47 and line 61 missing = after colspan so it produces colspan"1" instead of colspan="1"

    <td class=\"trow1\" colspan\"1\">

While testing this i have found something else that create invalid output in certain situation, but its not exacly related to this one so ill post it as other issue report. (should do in a fiew minutes)

This ones fixed, thanks.

WildcardSearch commented 11 years ago

I like all your ideas and appreciate all your feedback and hard work :)

WildcardSearch commented 11 years ago

I missed this for some reason but I just fixed that typo and will just wait for another release to push it here.