Re: [xwiki-devs] [xwiki-notifications] r24335 - in platform/web/trunk: standard/src/main/webapp/templates wysiwyg/src/main/java/com/xpn/xwiki/wysiwyg/client/editor wysiwyg/src/main/java/com/xpn/xwiki/wysiwyg/client/plugin/image/ui wysiwyg/src/main/java/com/xpn/xw - xwiki-devs@xwiki.org

9 Oct 2009

Hi,

...
   +    /**
 +     * {@inheritDoc}
 +     */
 +    public String getSpace()
 +    {
 +        return config.getParameter("space", "Main");
 +    }
 +
 +    /**
 +     * {@inheritDoc}
 +     */
 +    public String getWiki()
 +    {
 +        return config.getParameter("wiki", "xwiki");
 +    } 
 Actually I don't see a good reason for which I made these functions public.
 We
 should make them protected.

+1, I'll do it.

...

  +
 +    /**
 +     * {@inheritDoc}
 +     */
 +    protected String getFileHelpLabel()
 +    {
 +        return Strings.INSTANCE.importOfficeFileHelpLabel();
 +    }
 +
 +    /**
 +     * {@inheritDoc}
 +     */
 +    protected void onAttachmentUploaded(Attachment attach, final 
AsyncCallback<Boolean>  async)
  +    {
 +
 +        String fullPageName = getSpace() + "." + getPage(); 
 you should use the ResourceName class to build wiki names, since it's the
 client
 wiki reference serializer/deserializer. 

...
   +  
WysiwygService.Singleton.getInstance().officeToXHTML(fullPageName,
 getHTMLCleaningParams(),

Actually this code will go away when I introduce the new
officeToXHTML(Attachment....) method.

...
  +                    // Display the error and avoid
submit operation from
 continuing.
  +                   
displayError(thrown.getMessage());
 +                    async.onSuccess(false); 
 As a convention, we used the onFailure to signal an error on the server or
 in
 the server-client communication (like a GWT failure in the RPC call), while
 the
 submit async.onSuccess(false) is an "application error", a way in which a
 wizard
 step prevents the submit to be done because, for example, the user didn't
 insert
 valid data in the wizard form and another chance is being given to him.

 Now, which are the cases in which the importer service fails (throws
 exception)?
 is it correct to do this conversion here (from a server error / exception
 to a
 "validation" error)? or you should rather call async.onFailure()? this only
 depends on the logic of the service function failure.

Thanks. I will look into this and fix it.

...
  +     */
  +    public EnumSet<NavigationDirection> 
getValidDirections()
 +    {
 +        return EnumSet.of(NavigationDirection.FINISH); 
 We also return CANCEL here, even if a button for it is not displayed and
 it's
 always possible (through the dialog's close button), but in case we will
 ever
 want to display a button, or want to prevent the cancel in certain steps.

Ok, I will fix this.

...

  +    public void init(Object data,
AsyncCallback<  ?>  cb)
 +    {
 +        textArea.setHTML("");
 +        textArea.setFocus(true); 
 I seriously doubt this works, you should a FocusCommand(), look at its
 comment
 for reasons.

Ok.

...

  +    protected Map<String, String> 
getHTMLCleaningParams()
 +    {
 +        Map<String, String>  params = new HashMap<String, String>();
 +        params.put("filterStyles", "strict");
 +        // For Office2007: Office2007 generates an xhtml document (when  copied)
which has attributes and tags of
  +        // several namespaces. But the document
itself doesn't contain  the namespace definitions, which causes
  +        // the HTMLCleaner (the DomSerializer)
to fail while performing  it's operations. As a workaround we
  +        // force HTMLCleaner to avoid parsing of
namespace information.
 +        params.put("namespacesAware", Boolean.toString(false));
 +        return params;
 +    } 
 Is this the same function as in the other class?

Yes, I am reluctant to introduce a super class or a utility class to wrap up
this functionality. Not sure if I should do it or not.

...
  +        return requestedStep;

 Maybe we could create a super class for this, it's the same everywhere.

+1.

...
  Also,

 svn propset svn:keywords "Author Id Revision HeadURL" <new-file-path>

 for all new added files.

 check

http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HSubvers…
 for how to configure this for your svn client. Note that, even if correctly
 configured, the settings will not be considered by the eclipse plugin (I
 didn't
 manage to make them work), you'll have to do a command-line svn add for
 that to
 work right.

Ok, thanks for the tip.

- Asiri