Re: [wicket] branch csp updated: WICKET-6725: replace display:none by wicket--hidden css class

Sven Meier Tue, 14 Jan 2020 11:47:13 -0800

Hi,

>Using the hidden attribute is nooption either,
>because it is overridden by any css that sets display.


why is that a no-option, hidden elements can be styled too:

|[hidden] { display: none !important} |

Do you mean FormComponentFeedbackBorder's "color:red;" style?

I wouldn't want that to be located in a 'kitchen-sink' CSS file in thefuture.


Have fun
Sven||
||


On 14.01.20 20:31, Emond Papegaaij wrote:

Hi,

Sven, we do need styling to hide components.
Component.renderPlaceholderTag renders an empty tag for whatever it
reads from your markup. That fact that it is empty, does not mean it
is invisible. For example, in my application I had empty list-items
(li) popping up all over the place. Using the hidden attribute is no
option either, because it is overridden by any css that sets display.
We need something that takes precedence over most styling and hides
the component for sure.

Maxim, I didn't know about the application wide IHeaderContributors.
I'll give that a try, it seems cleaner to me than overriding
renderHead in Page.

About the contents of the stylesheet, it will probably not contain
much styling, but it will be more than just wicket--hidden. For
example, the styling for the form submit handling markup needs to go
there as well. I also found some other places with inline styling,
like FormComponentFeedbackBorder. We need to decide what to do with
those as well.

Best regards,
Emond

On Tue, Jan 14, 2020 at 7:19 PM Sven Meier <s...@meiers.net> wrote:

Hi Maxim,

an empty div with hidden inputs does not take any space.

Try it with wicket-examples.

Have fun
Sven

Am 14. Januar 2020 18:07:23 MEZ schrieb Maxim Solodovnik <solomax...@gmail.com>:

I thought the main idea is to keep placeholder for any element both
invisible and "take no space"
hidden attribute might work instead of `display: none`
The latter also hides all descendants [1], so I guess it should be
preferable

[1] https://developer.mozilla.org/en-US/docs/Web/CSS/display

On Wed, 15 Jan 2020 at 00:01, Sven Meier <s...@meiers.net> wrote:

Hi,

do we really need this CSS?

AFAIK the 'hidden' markup from Form and Component placeholders do not
need any styling actually, they look fine without it.
We should add a CSS class to the markup of course, but leave its

styling

to each project.

Cases in wicket-examples (UploadProgressBar, deprecated ModalWindow

and

AjaxIndicatorAppender) could switch to a different solution (e.g.

their

own CSS).

If wicket-base.css contains ".wicket--hidden" only, the effort isn't
worth it IMHO.

Sven


On 14.01.20 16:23, Maxim Solodovnik wrote:

For example it can be added as
`Application.get().getHeaderContributorListeners()`

On Tue, 14 Jan 2020 at 16:03, Maxim Solodovnik

<solomax...@gmail.com>

wrote:

`!important` is not the silver bullet (as well as inline style)

`renderHead` is not as important as `onConfigure`, so I believe it
shouldn't be made mandatory
Maybe there is some `hackish` way to inject this css only once for

any

component hierarchy?

On Tue, 14 Jan 2020 at 15:55, Emond Papegaaij <

emond.papega...@gmail.com>

wrote:

Rendering components without a page will indeed require you to

include

the core css file yourself. I think that's better than adding the

css

file with every component, as that will impose a massive

overhead.

I've renamed the css file to wicket-core.css as suggested by

Martin.

The idea is to collect all styling used with wicket-core in this

css

file. I do not like the idea to use a behavior for adding the
stylesheet, as that will increase the size of every page. Maybe a
temporary behavior can be used, but that will need to be re-added

on

every render then. Maybe, we can add a check to super.renderHead,

like

we do with onConfgure and onInitialize?

Using the hidden attribute is not a good idea, for the same

reason as

I chose to use !important in the css file: any matching css rule

that

is more specify will otherwise override the visibility and cause

the

component to be visible anyway (like display: flex).

Best regards,
Emond

On Tue, Jan 14, 2020 at 9:10 AM Maxim Solodovnik

<solomax...@gmail.com

wrote:

The problem as I see it
The component will be rendered without page (and without CSS

file

itself)

so element with this class will actually be visible ....

On Tue, 14 Jan 2020 at 15:08, Martin Grigorov

<mgrigo...@apache.org>

wrote:

On Tue, Jan 14, 2020 at 10:01 AM Maxim Solodovnik <

solomax...@gmail.com>

wrote:

An related question:
Will this code

`org.apache.wicket.core.util.string.ComponentRenderer.renderComponent(Component)`

work as expected?

This method will render the component with

class="wicket--hidden" on

its

HTML element. With TagTester you can verify
CSP checks are done only by the browsers.

On Tue, 14 Jan 2020 at 14:15, Maxim Solodovnik <

solomax...@gmail.com>

wrote:

Is this comment make sense:

https://github.com/apache/wicket/commit/6d91a6a9e5c1d955a53571f9fb0f76262ac5c5d2#r36784645

?

On Tue, 14 Jan 2020 at 14:13, Martin Grigorov <

mgrigo...@apache.org>

wrote:

On Tue, Jan 14, 2020 at 9:05 AM Maxim Solodovnik <

solomax...@gmail.com>

wrote:

Wasn't aware of `hidden` attribute
(and it seems to be widely supported

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden

Thanks for the reference, Maxim!

It says "elements that are descendants of a hidden element

are

still

active, which means that script elements can still execute

and

form

elements can still submit" and this is different than

display:

none.

So we should stick with CSS display!

Thanks :)

On Tue, 14 Jan 2020 at 14:01, Martin Grigorov <

mgrigo...@apache.org

wrote:

Hi,

1) I see that such CSS resource might be used for other

needs, not

just

this particular case but would it be an option to use

"hidden"

attribute

in

this case instead of CSS "display" ?

2) wicket-core.css instead of wicket-base.css ?

On Mon, Jan 13, 2020 at 9:43 PM <papega...@apache.org>

wrote:

This is an automated email from the ASF dual-hosted git

repository.

papegaaij pushed a commit to branch csp
in repository

https://gitbox.apache.org/repos/asf/wicket.git

The following commit(s) were added to refs/heads/csp by

this

push:

       new 6d91a6a  WICKET-6725: replace display:none by

wicket--hidden

css

class
6d91a6a is described below

commit 6d91a6a9e5c1d955a53571f9fb0f76262ac5c5d2
Author: Emond Papegaaij <emond.papega...@topicus.nl>
AuthorDate: Mon Jan 13 20:43:01 2020 +0100

      WICKET-6725: replace display:none by wicket--hidden

css

class

---
   .../src/main/java/org/apache/wicket/Component.java |  2

+-

   .../src/main/java/org/apache/wicket/Page.java      | 11

+++++++

   .../wicket/css/WicketBaseCSSResourceReference.java | 36
++++++++++++++++++++++
   .../java/org/apache/wicket/css/wicket-base.css     |  3

++

   .../apache/wicket/settings/ResourceSettings.java   | 31
+++++++++++++++++++
   .../ajax/markup/html/AjaxIndicatorAppender.java    |  2

+-

   6 files changed, 83 insertions(+), 2 deletions(-)

diff --git

a/wicket-core/src/main/java/org/apache/wicket/Component.java

b/wicket-core/src/main/java/org/apache/wicket/Component.java

index 9da8ec5..d2da23b 100644
---

a/wicket-core/src/main/java/org/apache/wicket/Component.java

+++

b/wicket-core/src/main/java/org/apache/wicket/Component.java

@@ -2367,7 +2367,7 @@ public abstract class Component
                  response.write(name);
                  response.write(" id=\"");
                  response.write(getAjaxRegionMarkupId());
-               response.write("\" style=\"display:none\"
data-wicket-placeholder=\"\"></");
+               response.write("\"

class=\"wicket--hidden\"

data-wicket-placeholder=\"\"></");
                  response.write(name);
                  response.write(">");
          }
diff --git

a/wicket-core/src/main/java/org/apache/wicket/Page.java

b/wicket-core/src/main/java/org/apache/wicket/Page.java
index 3f0f5b5..3d70ad8 100644
---

a/wicket-core/src/main/java/org/apache/wicket/Page.java

+++

b/wicket-core/src/main/java/org/apache/wicket/Page.java

@@ -24,10 +24,13 @@ import java.util.Set;

   import

org.apache.wicket.authorization.UnauthorizedActionException;

   import org.apache.wicket.core.util.lang.WicketObjects;
+import

org.apache.wicket.css.WicketBaseCSSResourceReference;

   import org.apache.wicket.feedback.FeedbackDelay;
   import org.apache.wicket.markup.MarkupException;
   import org.apache.wicket.markup.MarkupStream;
   import org.apache.wicket.markup.MarkupType;
+import org.apache.wicket.markup.head.CssHeaderItem;
+import org.apache.wicket.markup.head.IHeaderResponse;
   import org.apache.wicket.markup.html.WebPage;
   import

org.apache.wicket.markup.resolver.IComponentResolver;

   import org.apache.wicket.model.IModel;
@@ -1003,6 +1006,14 @@ public abstract class Page extends

MarkupContainer

                  }
          }

+       @Override
+       public void renderHead(IHeaderResponse response)
+       {
+               super.renderHead(response);
+               response.render(
+

CssHeaderItem.forReference(getApplication().getResourceSettings().getWicketBaseCSS()));

+       }
+
          /**
           * THIS METHOD IS NOT PART OF THE WICKET PUBLIC

API. DO

NOT

CALL.

           *
diff --git

a/wicket-core/src/main/java/org/apache/wicket/css/WicketBaseCSSResourceReference.java
b/wicket-core/src/main/java/org/apache/wicket/css/WicketBaseCSSResourceReference.java

new file mode 100644
index 0000000..9247216
--- /dev/null
+++

b/wicket-core/src/main/java/org/apache/wicket/css/WicketBaseCSSResourceReference.java

@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF)

under

one

or

more

+ * contributor license agreements.  See the NOTICE file

distributed

with

+ * this work for additional information regarding

copyright

ownership.

+ * The ASF licenses this file to You under the Apache

License,

Version

2.0

+ * (the "License"); you may not use this file except in

compliance

with

+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in

writing,

software

+ * distributed under the License is distributed on an

"AS

IS"

BASIS,

+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either

express

or

implied.
+ * See the License for the specific language governing

permissions

and

+ * limitations under the License.
+ */
+package org.apache.wicket.css;
+
+import

org.apache.wicket.request.resource.CssResourceReference;

+
+public final class WicketBaseCSSResourceReference

extends

CssResourceReference
+{
+       private static final long serialVersionUID =

6795863553105608280L;

+
+       private static final

WicketBaseCSSResourceReference

INSTANCE =

new

WicketBaseCSSResourceReference();
+
+       public static WicketBaseCSSResourceReference

get()

+       {
+               return INSTANCE;
+       }
+
+       private WicketBaseCSSResourceReference()
+       {
+

super(WicketBaseCSSResourceReference.class,

"wicket-base.css");
+       }
+}
diff --git

a/wicket-core/src/main/java/org/apache/wicket/css/wicket-base.css
b/wicket-core/src/main/java/org/apache/wicket/css/wicket-base.css

new file mode 100644
index 0000000..9bbdd63
--- /dev/null
+++

b/wicket-core/src/main/java/org/apache/wicket/css/wicket-base.css

@@ -0,0 +1,3 @@
+.wicket--hidden {
+       display: none!important;
+}
\ No newline at end of file
diff --git

a/wicket-core/src/main/java/org/apache/wicket/settings/ResourceSettings.java
b/wicket-core/src/main/java/org/apache/wicket/settings/ResourceSettings.java

index b76fbcd..68317f4 100644
---

a/wicket-core/src/main/java/org/apache/wicket/settings/ResourceSettings.java

+++

b/wicket-core/src/main/java/org/apache/wicket/settings/ResourceSettings.java

@@ -29,12 +29,14 @@ import

org.apache.wicket.core.util.resource.locator.IResourceStreamLocator;

   import

org.apache.wicket.core.util.resource.locator.ResourceStreamLocator;

   import

org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator;

   import org.apache.wicket.css.ICssCompressor;
+import

org.apache.wicket.css.WicketBaseCSSResourceReference;

   import

org.apache.wicket.javascript.IJavaScriptCompressor;

   import

org.apache.wicket.markup.head.PriorityFirstComparator;

   import

org.apache.wicket.markup.head.ResourceAggregator.RecordedHeaderItem;

   import

org.apache.wicket.markup.html.IPackageResourceGuard;

   import

org.apache.wicket.markup.html.SecurePackageResourceGuard;

   import org.apache.wicket.request.http.WebResponse;
+import

org.apache.wicket.request.resource.CssResourceReference;

   import

org.apache.wicket.request.resource.caching.FilenameWithVersionResourceCachingStrategy;

   import

org.apache.wicket.request.resource.caching.IResourceCachingStrategy;

   import

org.apache.wicket.request.resource.caching.NoOpResourceCachingStrategy;

@@ -172,6 +174,8 @@ public class ResourceSettings

implements

IPropertiesFactoryContext
                  false);

          private boolean encodeJSessionId = false;
+
+       private CssResourceReference wicketBaseCSS =
WicketBaseCSSResourceReference.get();

          /**
           * Configures Wicket's default

ResourceLoaders.<br>

@@ -770,4 +774,31 @@ public class ResourceSettings

implements

IPropertiesFactoryContext
                  this.encodeJSessionId =

encodeJSessionId;

                  return this;
          }
+
+       /**
+        * Returns the resource reference of the base

stylesheet

for

Wicket. This stylesheet contains
+        * some lowlevel styling used by Wicket.
+        *
+        * @return The resource reference of the base

stylesheet

for

Wicket.
+        */
+       public CssResourceReference getWicketBaseCSS()
+       {
+               return wicketBaseCSS;
+       }
+
+       /**
+        * Replaces the base stylesheet for Wicket.

Changes made

to

the

styling can break functionality
+        * like {@link

Component#setOutputMarkupPlaceholderTag(boolean)},

causing components that should
+        * not be visible to be displayed. Make sure the

replacement

stylesheet has matching definitions
+        * for the corresponding sections in the Wicket

version.

+        *
+        * @param wicketBaseCSS
+        *            The replacement styleheet.
+        * @return {@code this} object for chaining
+        */
+       public ResourceSettings

setWicketBaseCSS(CssResourceReference

wicketBaseCSS)
+       {
+               this.wicketBaseCSS = wicketBaseCSS;
+               return this;
+       }
   }
diff --git

a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxIndicatorAppender.java
b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxIndicatorAppender.java

index d0fee97..ab1c0a1 100644
---

a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxIndicatorAppender.java

+++

b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxIndicatorAppender.java

@@ -95,7 +95,7 @@ public class AjaxIndicatorAppender

extends

Behavior

                  super.afterRender(component);
                  final Response r =

component.getResponse();

-               r.write("<span style=\"display:none;\"

class=\"");

+               r.write("<span class=\"wicket--hidden\"

class=\"");

                  r.write(getSpanClass());
                  r.write("\" ");
                  r.write("id=\"");

--
WBR
Maxim aka solomax

--
WBR
Maxim aka solomax

--
WBR
Maxim aka solomax

--
WBR
Maxim aka solomax

--
WBR
Maxim aka solomax


--
WBR
Maxim aka solomax

Re: [wicket] branch csp updated: WICKET-6725: replace display:none by wicket--hidden css class

Reply via email to