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.

Sign up for GitHub

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

Jump to bottom

Fix baseUrl not working in include src attribute #1088

Merged
merged 1 commit into from
Mar 29, 2020
Merged

Fix baseUrl not working in include src attribute #1088

merged 1 commit into from
Mar 29, 2020

Conversation

ang-zeyu
Copy link
Contributor

@ang-zeyu ang-zeyu commented Mar 4, 2020 •
edited
Loading

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [x] Bug fix

Fixes #928
Requires #1087 ( removal of _rebaseReferenceForStaticIncludes ( neccessary ) and adding getParentSiteAbsolutePath ( standardisation ) ) merged

What is the rationale for this request?
To fix the case that the user wants to use {{baseUrl}} inside the src attribute of a <include/panel>.

What changes did you make? (Give an overview)

  • When resolving the source file path of the included path, in addition to checking whether the source file is url, we check if the src has a {{\s*baseUrl\s*}}[/\\] regex.
    If so, we recalculate the include source file path using the baseUrl of the current working file.
  • Added some functional tests to check for such cases
  • Added Markbind.unwrapIncludeSrc to the dynamic include build chain since it was missed before and my functional tests require it ( otherwise data-included-from would have my system's absolute file path, causing tests to fail )

Provide some example code that this change will affect:

...
if (baseUrlRegex.test(includePath)) {
  // The baseUrl has not been resolved during pre-processing, but we need the source file path
  const parentSitePath = urlUtils.getParentSiteAbsolutePath(context.cwf, config.rootPath, config.baseUrlMap);
  filePath = path.resolve(parentSitePath, includePath.replace(baseUrlRegex, ''));
} else {
...

Is there anything you'd like reviewers to focus on?
na

Testing instructions:

  • npm run test should pass
  • 2103 site should differ for **._include_.html files as Markbind.unwrapIncludeSrc was added. ( the wrapper div and spans with data-included-from should be deleted. )

Proposed commit message: (wrap lines at 72 characters)
Fix baseUrl not working in include src attribute

While pre-processing includes or panels, we have not yet resolved
the baseUrl of the src attribute.
This causes markbind to fail in finding the source file, since baseUrl
remains in the src attribute.

Let’s resolve the baseUrl for such cases first, allowing the user to use
the baseUrl attribute in include and panel src attributes.

@ang-zeyu ang-zeyu force-pushed the fix-base-url branch 4 times, most recently from 2a67704 to 9165b20 Compare March 9, 2020 10:30
@ang-zeyu
Copy link
Contributor Author

ang-zeyu commented Mar 15, 2020 •
edited
Loading

Can be reviewed now that #1087 has been merged!

@ang-zeyu ang-zeyu force-pushed the fix-base-url branch 4 times, most recently from 900881a to 0972aef Compare March 22, 2020 05:31
@ang-zeyu
Fix baseUrl not working in include src attribute
c39c354 
While pre-processing includes or panels, we have not yet resolved
the baseUrl of the src attribute.
This causes markbind to fail in finding the source file, since baseUrl
remains in the src attribute.

Let’s resolve the baseUrl for such cases first, allowing the user to use
the baseUrl attribute in include and panel src attributes.
@ang-zeyu ang-zeyu requested a review from openorclose March 23, 2020 15:08
@yamgent yamgent added this to the v2.12.1 milestone Mar 29, 2020
@yamgent yamgent merged commit 13dc7b7 into MarkBind:master Mar 29, 2020
marvinchin pushed a commit that referenced this pull request Apr 10, 2020
@ang-zeyu
Fix baseUrl not working in include src attribute (#1088)
a9f49ca 
While pre-processing includes or panels, we have not yet resolved
the baseUrl of the src attribute.
This causes markbind to fail in finding the source file, since baseUrl
remains in the src attribute.

Let’s resolve the baseUrl for such cases first, allowing the user to use
the baseUrl attribute in include and panel src attributes.
ang-zeyu added a commit to ang-zeyu/markbind that referenced this pull request Jun 10, 2020
@ang-zeyu
Refactor baseUrl into variablePreprocessor
1242ab1 
BaseUrl processing is done in a separate stage involving repeated and
recursive parsing / rendering of the content.

This decreases cohesiveness of variable processing, and also
performance due to the repeated parsing and rendering.
It also necessitates edge-case solutions such as that in MarkBind#1088 when we
need to resolve the baseUrl before the resolveBaseUrl stage has been
reached.

With a framework for variable processing now, let's move baseUrl
processing into it, solving the above said problems.

Furthermore, rendering of other variables containing html is dependent
on the extra htmlparser call in resolveBaseUrl.

Let's formally remove the need for this by using only the unescaped
nunjucks environment to render variables.
@ang-zeyu ang-zeyu mentioned this pull request Jun 10, 2020
Merged
ang-zeyu added a commit to ang-zeyu/markbind that referenced this pull request Jun 10, 2020
@ang-zeyu
Refactor baseUrl into variablePreprocessor
ada4d76 
BaseUrl processing is done in a separate stage involving repeated and
recursive parsing / rendering of the content.

This decreases cohesiveness of variable processing, and also
performance due to the repeated parsing and rendering.
It also necessitates edge-case solutions such as that in MarkBind#1088 when we
need to resolve the baseUrl before the resolveBaseUrl stage has been
reached.

With a framework for variable processing now, let's move baseUrl
processing into it, solving the above said problems.

Furthermore, rendering of other variables containing html is dependent
on the extra htmlparser call in resolveBaseUrl.

Let's formally remove the need for this by using only the unescaped
nunjucks environment to render variables.
ang-zeyu added a commit to ang-zeyu/markbind that referenced this pull request Jun 13, 2020
@ang-zeyu
Refactor baseUrl into variablePreprocessor
d019495 
BaseUrl processing is done in a separate stage involving repeated and
recursive parsing / rendering of the content.

This decreases cohesiveness of variable processing, and also
performance due to the repeated parsing and rendering.
It also necessitates edge-case solutions such as that in MarkBind#1088 when we
need to resolve the baseUrl before the resolveBaseUrl stage has been
reached.

With a framework for variable processing now, let's move baseUrl
processing into it, solving the above said problems.

Furthermore, rendering of other variables containing html is dependent
on the extra htmlparser call in resolveBaseUrl.

Let's formally remove the need for this by using only the unescaped
nunjucks environment to render variables.
ang-zeyu added a commit to ang-zeyu/markbind that referenced this pull request Jun 13, 2020
@ang-zeyu
Refactor baseUrl into variablePreprocessor
7249a46 
BaseUrl processing is done in a separate stage involving repeated and
recursive parsing / rendering of the content.

This decreases cohesiveness of variable processing, and also
performance due to the repeated parsing and rendering.
It also necessitates edge-case solutions such as that in MarkBind#1088 when we
need to resolve the baseUrl before the resolveBaseUrl stage has been
reached.

With a framework for variable processing now, let's move baseUrl
processing into it, solving the above said problems.

Furthermore, rendering of other variables containing html is dependent
on the extra htmlparser call in resolveBaseUrl.

Let's formally remove the need for this by using only the unescaped
nunjucks environment to render variables.
ang-zeyu added a commit to ang-zeyu/markbind that referenced this pull request Jun 22, 2020
@ang-zeyu
Refactor baseUrl into variablePreprocessor
cdcfed7 
BaseUrl processing is done in a separate stage involving repeated and
recursive parsing / rendering of the content.

This decreases cohesiveness of variable processing, and also
performance due to the repeated parsing and rendering.
It also necessitates edge-case solutions such as that in MarkBind#1088 when we
need to resolve the baseUrl before the resolveBaseUrl stage has been
reached.

With a framework for variable processing now, let's move baseUrl
processing into it, solving the above said problems.

Furthermore, rendering of other variables containing html is dependent
on the extra htmlparser call in resolveBaseUrl.

Let's formally remove the need for this by using only the unescaped
nunjucks environment to render variables.
ang-zeyu added a commit to ang-zeyu/markbind that referenced this pull request Jun 26, 2020
@ang-zeyu
Refactor baseUrl into variablePreprocessor
a633690 
BaseUrl processing is done in a separate stage involving repeated and
recursive parsing / rendering of the content.

This decreases cohesiveness of variable processing, and also
performance due to the repeated parsing and rendering.
It also necessitates edge-case solutions such as that in MarkBind#1088 when we
need to resolve the baseUrl before the resolveBaseUrl stage has been
reached.

With a framework for variable processing now, let's move baseUrl
processing into it, solving the above said problems.

Furthermore, rendering of other variables containing html is dependent
on the extra htmlparser call in resolveBaseUrl.

Let's formally remove the need for this by using only the unescaped
nunjucks environment to render variables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yamgent yamgent yamgent approved these changes

@openorclose openorclose Awaiting requested review from openorclose

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.13.0
Development

Successfully merging this pull request may close these issues.

{{baseUrl}} in boilerplates is not behaving as an absolute URL
2 participants
@ang-zeyu @yamgent