Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setting target Module Name when generating a block #1287

Closed
dbsdsun opened this issue May 18, 2015 · 8 comments
Closed

setting target Module Name when generating a block #1287

dbsdsun opened this issue May 18, 2015 · 8 comments

Comments

@dbsdsun
Copy link

dbsdsun commented May 18, 2015

Problem:

  1. One block may template files. The Module Name is used to determine the template file location.
  2. MoudleB_BlockB can extend ModuleA_BlockA by specification in di.xml as following:

In this case, the MoudleB_BlockB should use ModuleA as its module name, rather than MoudleB.

Solution:

Set target module name when generating a block as follwing. And this is last chance to set target module name.
in file lib/internal/Magento/Framework/View/Layout/Generator/Block.php, after line 157, add one line
line155. $block = $this->createBlock($className, $elementName, [
line156. 'data' => $this->evaluateArguments($data['arguments'])
line157. ]);
new line. $block->setModuleName($block->extractModuleName($className));
line158. if (!empty($attributes['template'])) {
line159. $block->setTemplate($attributes['template']);
line160. }

@anupdugar anupdugar added the PS label May 18, 2015
@paliarush
Copy link
Contributor

Hello @dbsdsun,

Looks like mentioned di.xml content is missing in the problem description. Please provide more details.

Thank you.

@dbsdsun
Copy link
Author

dbsdsun commented May 20, 2015

In magento 2, a block can be extended as following:

  1. add a preference in di.xml
  2. extend \Magento\Theme\Block\Html\Title class
    namespace Sample\HelloWorld\Block;

use Magento\Framework\View\Element\Template;

class HelloTitle extends \Magento\Theme\Block\Html\Title
{
public function getPageTitle()
{
return 'haha9999';
}
}

  1. HelloTitle->__toHtml() will locate template file with ModuleName
    public function getTemplateFile($template = null)
    {
    $params = ['module' => $this->getModuleName()];

To use the correct template file html/title.phtml, the ModuleName should be Magento_Theme. In the current Magento2, the ModuleName will be Sample_HelloWorld, which is wrong.

  1. When generating a block, the extended class is found with Magento\Framework\ObjectManager\Config->getPreference($type). But the ModuleName is not set to correct value (Magento_Theme, in my example).
    protected function generateBlock(
    Layout\ScheduledStructure $scheduledStructure,
    Layout\Data\Structure $structure,
    $elementName
    ) {
    .........................
    // create block
    $className = $attributes['class'];
    $block = $this->createBlock($className, $elementName, [
    'data' => $this->evaluateArguments($data['arguments'])
    ]);
    // NEEDS TO SET MODULENAME
    if (!empty($attributes['template'])) {
    $block->setTemplate($attributes['template']);
    }

@victorgugo
Copy link
Contributor

Hello, @dbsdsun

Thank you for interesting in Magento 2. Sorry, but from summary and description I couldn't catch what is the issue

  1. Could you please clarify what functionality is working wrong?
  2. Please, provide what problem you are solving, your steps, actual and expected behavior.
  3. It would be also good if you format your proposed code snippet for solution of this problem, beacuse now it's little bit not readable

Thanks in advance

@dbsdsun
Copy link
Author

dbsdsun commented May 22, 2015

Let me make it simple.
this is the function in file lib\internal\Magento\Framework\View\Layout\Generator\Block.php, I put some comments in it

    /**
     * Create block and set related data
     *
     * @param \Magento\Framework\View\Layout\ScheduledStructure $scheduledStructure
     * @param \Magento\Framework\View\Layout\Data\Structure $structure
     * @param string $elementName
     * @return \Magento\Framework\View\Element\AbstractBlock
     */
    protected function generateBlock(
        Layout\ScheduledStructure $scheduledStructure,
        Layout\Data\Structure $structure,
        $elementName
    ) {
        list(, $data) = $scheduledStructure->getElement($elementName);
        $attributes = $data['attributes'];

        if (!empty($attributes['group'])) {
            $structure->addToParentGroup($elementName, $attributes['group']);
        }

        // create block
        // setp 1. create a block object
        $className = $attributes['class'];
        $block = $this->createBlock($className, $elementName, [
            'data' => $this->evaluateArguments($data['arguments'])
        ]);

        // setp 2. set template (eg. "html/title.phtml"), if this block has template attribute
        if (!empty($attributes['template'])) {
            $block->setTemplate($attributes['template']);
        }

        // step 3. deal with attribute "ttl"
        if (!empty($attributes['ttl'])) {
            $ttl = (int)$attributes['ttl'];
            $block->setTtl($ttl);
        }
        return $block;
    }

When Magento2 locates a template file of a block, the attribute (eg. "html/title.phtml") is not enough. Magento2 also needs ModuleName of the block. Therefore, ModuleName also needs to be set in step 2. One line of code is needed.

$block->setModuleName($block->extractModuleName($className));

The slice of code should look like the following.

        // setp 2. set template (eg. "html/title.phtml"), if this block has template attribute
        if (!empty($attributes['template'])) {
            $block->setTemplate($attributes['template']);
            $block->setModuleName($block->extractModuleName($className));
        }

@dbsdsun
Copy link
Author

dbsdsun commented Jun 1, 2015

Your current unit test doesn't cover "override a block with preference in di.xml".
In the furture, if a block is overrided by another block, this issue will be seen.

@piotrekkaminski
Copy link
Contributor

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

magento-team pushed a commit that referenced this issue Jul 6, 2017
Fixed issues:

MAGETWO-59997 Shipping address duplicates if entered credit card was incorrect during checkout via Braintree
MAGETWO-70279 Issue with the config merging introduced for the Analytics integration
MAGETWO-70059 Order is not shown in customer account if its Status not visible on Storefront
MAGETWO-59801 [Performance Bug] Tax Rules Form unusable with large # of tax rates
MAGETWO-70280 Catalog Event can't save in the en_GB locale
MAGETWO-70314 [Github] cron:install issues #10040
MAGETWO-69750 An error occurs on the checkout after required custom address attribute added
MAGETWO-70490 Inconsistency between versions in module.xml and UbgradeSchema for CatalogRule Module
MAGETWO-66895 [Github] Cannot skip review page when order placed with Virtual/Gift Card product via PayPal Express Checkout #9042
@nikoelgatito
Copy link
Contributor

I encountered a similar issue when overriding the \Magento\Catalog\Block\Product\View block class in Magento 2.1.6.

There was to errors printing in the system.log as soon as the class was overriden:

[2017-08-12 19:43:12] main.CRITICAL: Invalid template file: 'product/view/review.phtml' in module: 'NameSpace_CustomModule_Override' block's name: 'product.info.review' [] []
[2017-08-12 19:43:12] main.CRITICAL: Invalid template file: 'product/view/form.phtml' in module: 'NameSpace_CustomModule_Override' block's name: 'product.info' [] []
[2017-08-12 19:43:12] main.CRITICAL: Invalid template file: 'product/view/addto.phtml' in module: 'NameSpace_CustomModule_Override' block's name: 'product.info.addto' [] []
[2017-08-12 19:43:12] main.CRITICAL: Invalid template file: 'product/view/mailto.phtml' in module: 'NameSpace_CustomModule_Override' block's name: 'product.info.mailto' [] []

To better understand the issue, what happens is that the template file locations set through the layout files are evaluated by Magento as relative paths and therefore as soon as we override a class, all relative template paths now fall under the module which is overriding the class.

Example:

Layout file: module-catalog/view/frontend/layout/catalog_product_view.xml
Line 85 | <block class="Magento\Catalog\Block\Product\View" name="product.info.addto" as="addto" template="product/view/addto.phtml">

Relative template path set via layout: product/view/addto.phtml

By default:

Absolute path generated by the getTemplateFile(...) function:
Magento_Catalog::product/view/addto.phtml

Once overriden:

Absolute path generated by the getTemplateFile(...) function: NameSpace_CustomModule::product/view/addto.phtml

This is a direct secondary effect of overriding a class.

Workaround:

The easiest workaround is to force the module_name back to Magento_Catalog as follow:
$this->setData('module_name', 'Magento_Catalog');

Comments:

Still as @dbsdsun mentioned, in my opinion the default Magento behavior when overriding a class should be to continue using the default absolute template paths unless specifically/separatelly overriden in the new class. I haven't tested the fix he shared above but there should be a simple enough way to make sure class overrides do not affect the initial template paths.

@korostii
Copy link
Contributor

the default Magento behavior when overriding a class should be to continue using the default absolute template paths

A better solution IMHO is to replace all the relative template paths there are in the core via layout XML, each one separately, which can be done with a regex.
Actually, it was done: see #1895.

Not sure though if it can be safely merged into the 2.1 branch as well. It might turn out be a backwards-incompatible change (there might be custom modules that rely on the old functionality in order to override the template and don't declare new path explicitly).

magento-team pushed a commit that referenced this issue Jan 25, 2018
 - Merge Pull Request magento-engcom/magento2ce#1287 from magento-engcom-team/magento2:batch-6-forwardport-2.3-develop
 - Merged commits:
   1. 7734bd1
   2. d96327d
   3. 8c735e5
   4. 1a11319
   5. 3011b5e
   6. 850e38c
   7. 24dcb91
   8. c574e46
   9. 8161593
   10. f06e173
   11. 66f9593
   12. d59be58
   13. 11f6dc4
   14. 4fe19c1
   15. 3d085f5
   16. d28ddba
   17. dcfa68c
   18. ff33a1b
   19. df7ef2b
   20. 2c7a113
   21. 596e260
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants