User tests: Successful: Unsuccessful:
Deprecate direct use of $_scripts
, $_script
and $_styleSheets
, $_style
.
Introduce Document::getScripts()
Document::getStyleSheets()
etc.
Apply the patch and make sure everything still works as before the patch.
If $_scripts
, $_script
and $_styleSheets
, $_style
mentioned somewhere, then it should be changed to Document::getScripts()
, Document::getScriptDeclarations()
, Document::getStyleSheets()
, Document::getStyleDeclarations()
.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
?
|
Labels |
Added:
?
|
If we start deprecating them, then please add the functions like getScript to the HTMLDocument class. We should not bind the generic Document class to HTML stuff.
Scripts and stylesheets are not unique to HTML output only. You can use stylesheets within XML documents, as an example.
But then it would be better to make a base class for script based documents.
It'd be better to use the decorator pattern in all honesty because there are a number of properties and methods that do not apply to all output formats, but then you are forcing a lot of downstream code to introduce instanceof Foo
or method_exists()
checks for no real reason (and then you have to justify what behaviors should exist in something like the raw document, which doesn't align with a "proper" response format). The time for introducing that type of distinguishing logic was 10 years ago, the ship has sailed on building a Document API where someone makes an arbitrary decision to say that the scripts and stylesheets properties (as examples) will never be available in raw or JSON context.
(Or, just drop everything but HTML from core like I've lightly suggested a number of times because nobody wants to make any non-HTML outputs proper first class citizens and everyone just assumes Joomla will always print HTML or bypasses core if they do non-HTML to the point where it doesn't matter how broken core is)
you should at least have even a related setter method.
getScriptDeclarations -> setScriptDeclarations
getScripts -> setScripts etc
I can add removeScript, removeStyleshet etc
All scripts/styleshets should be added via addScript() addStyleShet() etc
you should at least have even a related setter method.
getScriptDeclarations -> setScriptDeclarations
getScripts -> setScripts etcI can add removeScript, removeStyleshet etc
All scripts/styleshets should be added via addScript() addStyleShet() etc
Yes that would be perfect. This is something that has always been missing in the Document API and in past years developers have been forced to use unset on the public properties.
Yes that would be perfect.
I will add in the weekend
drone said
fatal: refusing to merge unrelated histories
I not sure that it related to the PR
hold on this, for now
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-07-09 10:56:43 |
Closed_By | ⇒ | Fedik |
This PR is incomplete and not the equivalent of having access to $_scripts, $_script and $_styleSheets, $_style.
99% of cases we want to access $_scripts or $_styleSheets to manipulate or remove some file such as:
unset($doc->_scripts[$script]);
unset($doc->_styleSheets[$sheet]);
So if you want to make these properties as 'protected' in a future release and have a getter method, you should at least have even a related setter method.
getScriptDeclarations -> setScriptDeclarations
getScripts -> setScripts
...etc...
As a developer i still want to continue to remove a loaded script or stylesheet from the document as it's possible now.
There are plenty of extensions doing this out there.