Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-9499

Javascript function dE does not correctly detect element visibility

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.6, 3.0.7, 3.0.7-PL1
    • Fix Version/s: 3.0.8-RC1
    • Component/s: ACP
    • Labels:
      None
    • Environment:
      PHP Version 5.2.6-3ubuntu4.5 - Tested with Google Chrome 5 Beta and Firefox 3.6

      Description

      After trying to make use of the javascript function dE() in the ACP, I discovered that it is outdated and will not correctly detect the visibility of the specified element correctly.
      In the ACP, the function dE is defined as such:

      /**
      * Set display of page element
      * s[-1,0,1] = hide,toggle display,show
      */
      function dE(n, s, type)
      {
      	if (!type)
      	{
      		type = 'block';
      	}
       
      	var e = document.getElementById(n);
      	if (!s)
      	{
      		s = (e.style.display == '') ? -1 : 1;
      	}
      	e.style.display = (s == 1) ? type : 'none';
      }
      

      In proSilver, the dE function is defined like so:

      /**
      * Set display of page element
      * s[-1,0,1] = hide,toggle display,show
      */
      function dE(n, s)
      {
      	var e = document.getElementById(n);
       
      	if (!s)
      	{
      		s = (e.style.display == '' || e.style.display == 'block') ? -1 : 1;
      	}
      	e.style.display = (s == 1) ? 'block' : 'none';
      }
      

      The main difference between the function declarations is that of this snippet:

       || e.style.display == 'block'


      Adding this into the ACP variant of the function corrected the problem for me.

      At the moment, I am about to make a quick patch via git and submit a pull request, I'll reply back to this bug when I am done.

        Activity

        Hide
        Desdenova Desdenova [X] (Inactive) added a comment -

        Patch is available here: http://github.com/Obsidian1510/phpbb3/commit/d891f3b957577384cefccf237d02ee5452702795

        Also, apparently my editor found some extra whitespace on the end of a few lines in that file.

        Show
        Desdenova Desdenova [X] (Inactive) added a comment - Patch is available here: http://github.com/Obsidian1510/phpbb3/commit/d891f3b957577384cefccf237d02ee5452702795 Also, apparently my editor found some extra whitespace on the end of a few lines in that file.
        Hide
        naderman Nils Adermann added a comment -

        Rather than checking for 'block' the acp function should check for type, since it allows other display types as well. Please also move the white space changes into a separate commit.

        Show
        naderman Nils Adermann added a comment - Rather than checking for 'block' the acp function should check for type, since it allows other display types as well. Please also move the white space changes into a separate commit.
        Hide
        Desdenova Desdenova [X] (Inactive) added a comment -

        Alright, done.
        http://github.com/Obsidian1510/phpbb3/tree/bug/9499

        This is three commits in total; one commit contains the fix, one gets the function dE in adm/style/install_header.html to match the fixed version, and the last cleans up extra whitespace in both files I was working with.

        http://github.com/Obsidian1510/phpbb3/commit/52b7cfd054a81a9a50c99467691bd2d017951644
        http://github.com/Obsidian1510/phpbb3/commit/8865dfc8090c916b359d761f08ea34471cc4ff54
        http://github.com/Obsidian1510/phpbb3/commit/c85c25eb39a2b7f34945f1b66559ebb66c4c358d

        Show
        Desdenova Desdenova [X] (Inactive) added a comment - Alright, done. http://github.com/Obsidian1510/phpbb3/tree/bug/9499 This is three commits in total; one commit contains the fix, one gets the function dE in adm/style/install_header.html to match the fixed version, and the last cleans up extra whitespace in both files I was working with. http://github.com/Obsidian1510/phpbb3/commit/52b7cfd054a81a9a50c99467691bd2d017951644 http://github.com/Obsidian1510/phpbb3/commit/8865dfc8090c916b359d761f08ea34471cc4ff54 http://github.com/Obsidian1510/phpbb3/commit/c85c25eb39a2b7f34945f1b66559ebb66c4c358d
        Hide
        A_Jelly_Doughnut A_Jelly_Doughnut added a comment -

        This is one of dozens of discrepancies between the editor.js file included with prosilver, the editor.js file with subsilver2, the editor.js file in the /adm/ directory, and the /install/ directory.

        IMO these should all be fixed. The question is whether we can assume that the prosilver edition is canonical.

        Show
        A_Jelly_Doughnut A_Jelly_Doughnut added a comment - This is one of dozens of discrepancies between the editor.js file included with prosilver, the editor.js file with subsilver2, the editor.js file in the /adm/ directory, and the /install/ directory. IMO these should all be fixed. The question is whether we can assume that the prosilver edition is canonical.

          People

          • Assignee:
            A_Jelly_Doughnut A_Jelly_Doughnut
            Reporter:
            Desdenova Desdenova [X] (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development