Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.2
    • Fix Version/s: 1.3.5
    • Component/s: wicket
    • Labels:
      None

      Description

      I add a component to its container when the page is initialized, BEFORE
      it is rendered and before page model finished to initialize.
      if log4j is configured to enable wicket debug, the container toString()
      is called.
      This method invokes isVisible() of the container.
      I have overridden isVisible(), to query the page model before deciding
      whether the component is actually visible.
      I assumed this method is called when the component is rendered, and by
      that time the model is completely initialized.
      However, since it was called before component rendering, my code failed
      and the page failed to be created.
      To sum up, toString() of Component should be safe.

        Activity

        Hide
        Juergen Donnerstag added a comment -

        IMO the output of Component.toString(false) is too simple. Which is probably why MarkupContainer.toString(detailed) calls super.toString(true).

        i think we should

        • make Component.toString(false) more meaningful
        • and change MarkupContainer.toString(detailed) to call super.toString(detailed)

        Since most people will use toString(), they are with the not-so-detailed output anyway.

        Show
        Juergen Donnerstag added a comment - IMO the output of Component.toString(false) is too simple. Which is probably why MarkupContainer.toString(detailed) calls super.toString(true). i think we should make Component.toString(false) more meaningful and change MarkupContainer.toString(detailed) to call super.toString(detailed) Since most people will use toString(), they are with the not-so-detailed output anyway.
        Hide
        Alastair Maw added a comment -

        Hmmm, this is still an issue in trunk, now I look. I think toString() on Component should call toString(false). Worse, toString(false) on MarkupContainer calls super.toString(true) regardless.

        It's difficult to get Eclipse to tell me what calls .toString()

        I guess we should just turn detailed output off and call it explicitly when we find we need it, or when people whinge.

        Show
        Alastair Maw added a comment - Hmmm, this is still an issue in trunk, now I look. I think toString() on Component should call toString(false). Worse, toString(false) on MarkupContainer calls super.toString(true) regardless. It's difficult to get Eclipse to tell me what calls .toString() I guess we should just turn detailed output off and call it explicitly when we find we need it, or when people whinge.
        Hide
        Eelco Hillenius added a comment -

        I think that the thing that is wrong here is that toString calls this method with detailed == true. Why don't we make that false explicitly use this method with detailed == true when we use it for e.g. printing out the component etc?

        Show
        Eelco Hillenius added a comment - I think that the thing that is wrong here is that toString calls this method with detailed == true. Why don't we make that false explicitly use this method with detailed == true when we use it for e.g. printing out the component etc?
        Hide
        Nili Adoram added a comment -

        the same code appears in 1.2.2 as well.
        the page is already set, but the model is not completely initialized.
        This is probably my fault and I should reverse the order.
        However, toString() should not fail Container.add() .

        Show
        Nili Adoram added a comment - the same code appears in 1.2.2 as well. the page is already set, but the model is not completely initialized. This is probably my fault and I should reverse the order. However, toString() should not fail Container.add() .
        Hide
        Johan Compagner added a comment -

        in 1.3 its already safe:

        if (detailed)
        {
        final Page page = findPage();
        if (page == null)

        { return new StringBuffer("[Component id = ").append(getId()).append( ", page = <No Page>, path = ").append(getPath()).append(".").append( Classes.simpleName(getClass())).append("]").toString(); }

        else

        { return new StringBuffer("[Component id = ").append(getId()).append(", page = ") .append(getPage().getClass().getName()).append(", path = ").append( getPath()).append(".").append(Classes.simpleName(getClass())) .append(", isVisible = ").append((isRenderAllowed() && isVisible())) .append(", isVersioned = ").append(isVersioned()).append("]").toString(); }

        }
        else

        { return "[Component id = " + getId() + "]"; }

        so if we don't find a page many things are not called.

        Show
        Johan Compagner added a comment - in 1.3 its already safe: if (detailed) { final Page page = findPage(); if (page == null) { return new StringBuffer("[Component id = ").append(getId()).append( ", page = <No Page>, path = ").append(getPath()).append(".").append( Classes.simpleName(getClass())).append("]").toString(); } else { return new StringBuffer("[Component id = ").append(getId()).append(", page = ") .append(getPage().getClass().getName()).append(", path = ").append( getPath()).append(".").append(Classes.simpleName(getClass())) .append(", isVisible = ").append((isRenderAllowed() && isVisible())) .append(", isVersioned = ").append(isVersioned()).append("]").toString(); } } else { return "[Component id = " + getId() + "]"; } so if we don't find a page many things are not called.

          People

          • Assignee:
            Igor Vaynberg
            Reporter:
            Nili Adoram
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development