Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: Release Branch 11.04, Release Branch 12.04, Trunk
    • Component/s: content
    • Labels:
      None

      Description

      Tree rendering is missing labels -
      For example have a look at content tree -

      https://demo-trunk.ofbiz.apache.org/content/control/ViewCompDocTemplateTree?rootContentId=CDT1201

      Labels with tree node are missing.

      1. OFBIZ-5313_after.gif
        103 kB
        Sumit Pandit
      2. OFBIZ-5313_before.gif
        117 kB
        Sumit Pandit
      3. OFBIZ-5313-fixed.png
        78 kB
        Adrian Crum
      4. OFBIZ-5313-TreeLabels.patch
        3 kB
        Sumit Pandit
      5. OFBIZ-5313-TreeLabels.patch
        1 kB
        Sumit Pandit

        Activity

        Hide
        sumitp Sumit Pandit added a comment -

        Please find as-is and to-be screenshots with attachment.

        Show
        sumitp Sumit Pandit added a comment - Please find as-is and to-be screenshots with attachment.
        Hide
        sumitp Sumit Pandit added a comment -

        Attaching patch to fix the issue.

        Show
        sumitp Sumit Pandit added a comment - Attaching patch to fix the issue.
        Hide
        adrianc@hlmksw.com Adrian Crum added a comment -

        Please do not modify widget model state in the renderer - it is not thread safe.

        Remember that a single model instance is shared by multiple threads, and the renderer instance is per thread.

        Show
        adrianc@hlmksw.com Adrian Crum added a comment - Please do not modify widget model state in the renderer - it is not thread safe. Remember that a single model instance is shared by multiple threads, and the renderer instance is per thread.
        Hide
        sumitp Sumit Pandit added a comment -

        Thanks Adrian for your review, please have a look at modified patch.

        Show
        sumitp Sumit Pandit added a comment - Thanks Adrian for your review, please have a look at modified patch.
        Hide
        adrianc@hlmksw.com Adrian Crum added a comment -

        That looks better, but it seems a bit hackish. I understand it is the easiest way to work around the thread safety issue, and you are fighting against a poor design (the tree widget design started off entirely thread-unsafe, and we've had to fix it many times).

        If no one beats me to it, I will commit it when I have more time.

        Show
        adrianc@hlmksw.com Adrian Crum added a comment - That looks better, but it seems a bit hackish. I understand it is the easiest way to work around the thread safety issue, and you are fighting against a poor design (the tree widget design started off entirely thread-unsafe, and we've had to fix it many times). If no one beats me to it, I will commit it when I have more time.
        Hide
        arunpati Arun Patidar added a comment -

        I have verified attached patch and its working fine.

        Steps to test:
        1) Go to CONTENT.
        2) Now select the CompDoc Menu.
        3) The next step is to click on find to generate the list.
        4) Select the contentId CDT1201 and click on tree button.
        5) Now the tree rendered is missing labels.

        Show
        arunpati Arun Patidar added a comment - I have verified attached patch and its working fine. Steps to test: 1) Go to CONTENT. 2) Now select the CompDoc Menu. 3) The next step is to click on find to generate the list. 4) Select the contentId CDT1201 and click on tree button. 5) Now the tree rendered is missing labels.
        Hide
        toashishvijay Ashish Vijaywargiya added a comment -

        Thanks Sumit for creating the issue and providing the patch for the same. Your changes are committed at following revisions:

        trunk - 1646969
        R13.07 - 1646970
        R12.04 - 1646971

        Show
        toashishvijay Ashish Vijaywargiya added a comment - Thanks Sumit for creating the issue and providing the patch for the same. Your changes are committed at following revisions: trunk - 1646969 R13.07 - 1646970 R12.04 - 1646971
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Please Ashish change the "Fix versions" to unreleased ones.

        Show
        jacques.le.roux Jacques Le Roux added a comment - Please Ashish change the "Fix versions" to unreleased ones.
        Hide
        adrianc@hlmksw.com Adrian Crum added a comment -

        Reopening this because the attached patches are the wrong approach.

        Show
        adrianc@hlmksw.com Adrian Crum added a comment - Reopening this because the attached patches are the wrong approach.
        Hide
        adrianc@hlmksw.com Adrian Crum added a comment -

        I spent a lot of time researching this issue and I came to the conclusion that the attached patches do not fix the problem. Instead, they make things worse.

        If you look carefully at the "before" screenshot, you see what appears to be an empty tree and below it is a series of menu lines. The patch attempts to fix the problem by adding text to the empty tree. But that doesn't fix the problem because that empty tree is supposed to contain the menu lines displayed below it. In other words, the tree renderer is seriously broken.

        Show
        adrianc@hlmksw.com Adrian Crum added a comment - I spent a lot of time researching this issue and I came to the conclusion that the attached patches do not fix the problem. Instead, they make things worse. If you look carefully at the "before" screenshot, you see what appears to be an empty tree and below it is a series of menu lines. The patch attempts to fix the problem by adding text to the empty tree. But that doesn't fix the problem because that empty tree is supposed to contain the menu lines displayed below it. In other words, the tree renderer is seriously broken.
        Hide
        adrianc@hlmksw.com Adrian Crum added a comment -

        Fixed in trunk rev 1649239.

        Show
        adrianc@hlmksw.com Adrian Crum added a comment - Fixed in trunk rev 1649239.
        Hide
        adrianc@hlmksw.com Adrian Crum added a comment -

        Fixed in R13 and R14. Attached screenshot shows how the tree is supposed to look.

        Show
        adrianc@hlmksw.com Adrian Crum added a comment - Fixed in R13 and R14. Attached screenshot shows how the tree is supposed to look.

          People

          • Assignee:
            adrianc@hlmksw.com Adrian Crum
            Reporter:
            sumitp Sumit Pandit
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development