You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2008/11/05 18:25:48 UTC
svn commit: r711632 - in /tapestry/tapestry5/trunk/tapestry-core/src:
main/java/org/apache/tapestry5/internal/services/
main/java/org/apache/tapestry5/services/
main/resources/org/apache/tapestry5/internal/services/
test/java/org/apache/tapestry5/inter...
Author: hlship
Date: Wed Nov 5 09:25:40 2008
New Revision: 711632
URL: http://svn.apache.org/viewvc?rev=711632&view=rev
Log:
TAP5-184: Improve error reporting when a javascript asset is intended to be included on page which has no <html> element
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinker.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.java
tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/internal/services/no_body_element.txt
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinker.java?rev=711632&r1=711631&r2=711632&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinker.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinker.java Wed Nov 5 09:25:40 2008
@@ -22,7 +22,7 @@
{
/**
* Adds a link to load a script. Scripts will be loaded only once. The <script> elements will be added at the
- * bottom of the <body> element.
+ * top or bottom of the <body> element (the location is configurable).
*/
void addScriptLink(String scriptURL);
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java?rev=711632&r1=711631&r2=711632&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java Wed Nov 5 09:25:40 2008
@@ -76,15 +76,16 @@
{
Element root = document.getRootElement();
- // This can happen due to a catastrophic rendering error, such as a missing page template.
+ // If the document failed to render entirely, that's a different problem and is reported elsewhere.
if (root == null) return;
// This only applies when the document is an HTML document. This may need to change in the
// future, perhaps configurable, to allow for html and xhtml and perhaps others. Does SVG
// use stylesheets?
- if (!root.getName().equals("html")) return;
+ if (!root.getName().equals("html"))
+ throw new RuntimeException(ServicesMessages.documentMissingHTMLRoot());
if (!stylesheets.isEmpty())
addStylesheetsToHead(root, includedStylesheets);
@@ -96,7 +97,9 @@
{
Element body = root.find("body");
- if (body == null) return;
+ // Create the body element is it is somehow missing.
+
+ if (body == null) body = root.element("body");
// TAPESTRY-2364
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java?rev=711632&r1=711631&r2=711632&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java Wed Nov 5 09:25:40 2008
@@ -417,4 +417,9 @@
{
return MESSAGES.format("event-not-handled", eventName, element.getCompleteId());
}
+
+ static String documentMissingHTMLRoot()
+ {
+ return MESSAGES.get("document-missing-html-root");
+ }
}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java?rev=711632&r1=711631&r2=711632&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java Wed Nov 5 09:25:40 2008
@@ -95,8 +95,6 @@
private final RequestGlobals requestGlobals;
- private final ActionRenderResponseGenerator actionRenderResponseGenerator;
-
private final EnvironmentalShadowBuilder environmentalBuilder;
@@ -130,8 +128,6 @@
ThreadLocale threadLocale,
- ActionRenderResponseGenerator actionRenderResponseGenerator,
-
EnvironmentalShadowBuilder environmentalBuilder)
{
this.pipelineBuilder = pipelineBuilder;
@@ -146,7 +142,6 @@
this.request = request;
this.response = response;
this.threadLocale = threadLocale;
- this.actionRenderResponseGenerator = actionRenderResponseGenerator;
this.environmentalBuilder = environmentalBuilder;
}
@@ -1457,8 +1452,9 @@
/**
* Adds page render filters, each of which provides an {@link org.apache.tapestry5.annotations.Environmental}
- * service. Filters often provide {@link Environmental} services needed by components as they render. <dl>
- * <dt>PageRenderSupport</dt> <dd>Provides {@link org.apache.tapestry5.RenderSupport}</dd>
+ * service. Filters often provide {@link org.apache.tapestry5.annotations.Environmental} services needed by
+ * components as they render. <dl> <dt>DocumentLinker</dt> <dd>Provides {@link org.apache.tapestry5.internal.services.DocumentLinker}
+ * <dt>RenderSupport</dt> <dd>Provides {@link org.apache.tapestry5.RenderSupport}</dd>
* <dt>ClientBehaviorSupport</dt> <dd>Provides {@link org.apache.tapestry5.internal.services.ClientBehaviorSupport}</dd>
* <dt>Heartbeat</dt> <dd>Provides {@link org.apache.tapestry5.services.Heartbeat}</dd>
* <dt>DefaultValidationDecorator</dt> <dd>Provides {@link org.apache.tapestry5.ValidationDecorator} (as an instance
@@ -1484,12 +1480,28 @@
final AssetSource assetSource)
{
- MarkupRendererFilter pageRenderSupport = new MarkupRendererFilter()
+ MarkupRendererFilter documentLinker = new MarkupRendererFilter()
{
public void renderMarkup(MarkupWriter writer, MarkupRenderer renderer)
{
DocumentLinkerImpl linker = new DocumentLinkerImpl(productionMode, scriptsAtTop);
+ environment.push(DocumentLinker.class, linker);
+
+ renderer.renderMarkup(writer);
+
+ environment.pop(DocumentLinker.class);
+
+ linker.updateDocument(writer.getDocument());
+ }
+ };
+
+ MarkupRendererFilter renderSupport = new MarkupRendererFilter()
+ {
+ public void renderMarkup(MarkupWriter writer, MarkupRenderer renderer)
+ {
+ DocumentLinker linker = environment.peekRequired(DocumentLinker.class);
+
RenderSupportImpl support = new RenderSupportImpl(linker, symbolSource, assetSource,
// Core scripts added to any page that uses scripting
@@ -1508,11 +1520,9 @@
renderer.renderMarkup(writer);
- support.commit();
-
- linker.updateDocument(writer.getDocument());
-
environment.pop(RenderSupport.class);
+
+ support.commit();
}
};
@@ -1570,7 +1580,8 @@
};
- configuration.add("RenderSupport", pageRenderSupport);
+ configuration.add("DocumentLinker", documentLinker, "before:RenderSupport");
+ configuration.add("RenderSupport", renderSupport);
configuration.add("ClientBehaviorSupport", clientBehaviorSupport, "after:RenderSupport");
configuration.add("Heartbeat", heartbeat, "after:RenderSupport");
configuration.add("DefaultValidationDecorator", defaultValidationDecorator, "after:Heartbeat");
@@ -1578,10 +1589,8 @@
/**
- * Contributes {@link PartialMarkupRendererFilter}s used when rendering a partial Ajax response. This is an analog
- * to {@link #contributeMarkupRenderer(org.apache.tapestry5.ioc.OrderedConfiguration, boolean,
- * org.apache.tapestry5.Asset, org.apache.tapestry5.Asset, ValidationMessagesSource,
- * org.apache.tapestry5.ioc.services.SymbolSource, AssetSource)} } and overlaps it to some degree. <dl> <dt>
+ * Contributes {@link PartialMarkupRendererFilter}s used when rendering a partial Ajax response. <dl>
+ * <dt>DocumentLinker <dd>Provides {@link org.apache.tapestry5.internal.services.DocumentLinker} <dt>
* PageRenderSupport </dt> <dd>Provides {@link org.apache.tapestry5.RenderSupport}</dd>
* <dt>ClientBehaviorSupport</dt> <dd>Provides {@link org.apache.tapestry5.internal.services.ClientBehaviorSupport}</dd>
* <dt>Heartbeat</dt> <dd>Provides {@link org.apache.tapestry5.services.Heartbeat}</dd>
@@ -1599,7 +1608,24 @@
final ValidationMessagesSource validationMessagesSource)
{
- PartialMarkupRendererFilter pageRenderSupport = new PartialMarkupRendererFilter()
+ PartialMarkupRendererFilter documentLinker = new PartialMarkupRendererFilter()
+ {
+ public void renderMarkup(MarkupWriter writer, JSONObject reply, PartialMarkupRenderer renderer)
+ {
+ PartialMarkupDocumentLinker linker = new PartialMarkupDocumentLinker();
+
+ environment.push(DocumentLinker.class, linker);
+
+ renderer.renderMarkup(writer, reply);
+
+ environment.pop(DocumentLinker.class);
+
+ linker.commit(reply);
+ }
+ };
+
+
+ PartialMarkupRendererFilter renderSupport = new PartialMarkupRendererFilter()
{
public void renderMarkup(MarkupWriter writer, JSONObject reply, PartialMarkupRenderer renderer)
{
@@ -1609,7 +1635,7 @@
IdAllocator idAllocator = new IdAllocator(namespace);
- PartialMarkupDocumentLinker linker = new PartialMarkupDocumentLinker();
+ DocumentLinker linker = environment.peekRequired(DocumentLinker.class);
RenderSupportImpl support = new RenderSupportImpl(linker, symbolSource, assetSource,
idAllocator);
@@ -1621,8 +1647,6 @@
support.commit();
environment.pop(RenderSupport.class);
-
- linker.commit(reply);
}
};
@@ -1680,7 +1704,8 @@
};
- configuration.add("RenderSupport", pageRenderSupport);
+ configuration.add("DocumentLinker", documentLinker, "before:RenderSupport");
+ configuration.add("RenderSupport", renderSupport);
configuration.add("ClientBehaviorSupport", clientBehaviorSupport, "after:RenderSupport");
configuration.add("Heartbeat", heartbeat, "after:RenderSupport");
configuration.add("DefaultValidationDecorator", defaultValidationDecorator, "after:Heartbeat");
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties?rev=711632&r1=711631&r2=711632&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties Wed Nov 5 09:25:40 2008
@@ -94,4 +94,5 @@
parameter-binding-must-not-be-empty=Parameter '%s' must have a non-empty binding.
no-such-method=Class %s does not contain a method named '%s()'.
forbid-instantiate-component-class=Component class %s may not be instantiated directly. You should use an @InjectPage or @InjectComponent annotation instead.
-event-not-handled=Request event '%s' (on component %s) was not handled; you must provide a matching event handler method in the component or in one of its containers.
\ No newline at end of file
+event-not-handled=Request event '%s' (on component %s) was not handled; you must provide a matching event handler method in the component or in one of its containers.
+document-missing-html-root=The rendered document does not contain a root <html> element, which is necessary for the linking of JavaScript and stylesheet resources.
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.java?rev=711632&r1=711631&r2=711632&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.java Wed Nov 5 09:25:40 2008
@@ -26,8 +26,8 @@
assertEquals(document.toString(), readFile(file));
}
- @Test
- public void do_nothing_if_no_body()
+ @Test(expectedExceptions = RuntimeException.class)
+ public void exception_if_missing_html_root_element()
{
Document document = new Document();
@@ -39,8 +39,21 @@
linker.addScript("doSomething();");
linker.updateDocument(document);
+ }
+
+ @Test
+ public void missing_root_element_is_a_noop()
+ {
+ Document document = new Document();
- assertEquals(document.toString(), "<not-html>not an HTML document</not-html>");
+ DocumentLinkerImpl linker = new DocumentLinkerImpl(true, false);
+
+ linker.addScript("foo.js");
+ linker.addScript("doSomething();");
+
+ // No root element is not an error, even though there's work to do.
+ // The failure to render is reported elsewhere.
+ linker.updateDocument(document);
}
@Test
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/internal/services/no_body_element.txt
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/internal/services/no_body_element.txt?rev=711632&r1=711631&r2=711632&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/internal/services/no_body_element.txt (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/internal/services/no_body_element.txt Wed Nov 5 09:25:40 2008
@@ -1,2 +1,2 @@
<?xml version="1.0"?>
-<html><notbody><p>Ready to be updated with scripts.</p></notbody></html>
\ No newline at end of file
+<html><notbody><p>Ready to be updated with scripts.</p></notbody><body><script src="foo.js" type="text/javascript"/></body></html>
\ No newline at end of file