Skip to content

Remove <base> tag, use hrefs prepended with base #2098

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

Merged
merged 5 commits into from
Jan 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions lib/src/dartdoc_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,9 @@ class DartdocOptionContext extends DartdocOptionContextBase
excludePackages.any((pattern) => name == pattern);

String get templatesDir => optionSet['templatesDir'].valueAt(context);

// TODO(jdkoren): temporary while we confirm href base behavior doesn't break important clients
bool get useBaseHref => optionSet['useBaseHref'].valueAt(context);
}

/// Instantiate dartdoc's configuration file and options parser with the
Expand Down Expand Up @@ -1635,6 +1638,14 @@ Future<List<DartdocOption>> createDartdocOptions() async {
'property, top_level_constant, top_level_property, typedef. Partial templates are '
'supported; they must begin with an underscore, and references to them must omit the '
'leading underscore (e.g. use {{>foo}} to reference the partial template _foo.html).'),
DartdocOptionArgOnly<bool>('useBaseHref', false,
help:
'Use <base href> in generated files (legacy behavior). This option '
'is temporary and support will be removed in the future. Use only '
'if the default behavior breaks links between your documentation '
'pages, and please file an issue on Github.',
negatable: false,
hide: true),
// TODO(jcollins-g): refactor so there is a single static "create" for
// each DartdocOptionContext that traverses the inheritance tree itself.
]
Expand Down
9 changes: 7 additions & 2 deletions lib/src/html/html_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,17 @@ class HtmlGeneratorOptions implements HtmlOptions {
@override
final String toolVersion;

@override
final bool useBaseHref;

HtmlGeneratorOptions(
{this.url,
this.relCanonicalPrefix,
this.faviconPath,
String toolVersion,
this.prettyIndexJson = false,
this.templatesDir})
this.templatesDir,
this.useBaseHref = false})
: this.toolVersion = toolVersion ?? 'unknown';
}

Expand All @@ -166,7 +170,8 @@ Future<List<Generator>> initGenerators(GeneratorContext config) async {
toolVersion: dartdocVersion,
faviconPath: config.favicon,
prettyIndexJson: config.prettyIndexJson,
templatesDir: config.templatesDir);
templatesDir: config.templatesDir,
useBaseHref: config.useBaseHref);

return [
await HtmlGenerator.create(
Expand Down
10 changes: 10 additions & 0 deletions lib/src/html/html_generator_instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ class HtmlGeneratorInstance {
});

String json = encoder.convert(indexItems);
if (!_options.useBaseHref) {
json = json.replaceAll(HTMLBASE_PLACEHOLDER, '');
}
_writer(path.join('categories.json'), '${json}\n');
}

Expand Down Expand Up @@ -118,6 +121,9 @@ class HtmlGeneratorInstance {
});

String json = encoder.convert(indexItems);
if (!_options.useBaseHref) {
json = json.replaceAll(HTMLBASE_PLACEHOLDER, '');
}
_writer(path.join('index.json'), '${json}\n');
}

Expand Down Expand Up @@ -412,6 +418,10 @@ class HtmlGeneratorInstance {
// Replaces '/' separators with proper separators for the platform.
String outFile = path.joinAll(filename.split('/'));
String content = template.renderString(data);

if (!_options.useBaseHref) {
content = content.replaceAll(HTMLBASE_PLACEHOLDER, data.htmlBase);
}
_writer(outFile, content,
element: data.self is Warnable ? data.self : null);
if (data.self is Indexable) _indexedElements.add(data.self as Indexable);
Expand Down
26 changes: 14 additions & 12 deletions lib/src/html/template_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:dartdoc/src/model/model.dart';
abstract class HtmlOptions {
String get relCanonicalPrefix;
String get toolVersion;
bool get useBaseHref;
}

abstract class TemplateData<T extends Documentable> {
Expand Down Expand Up @@ -38,6 +39,7 @@ abstract class TemplateData<T extends Documentable> {
T get self;
String get version => htmlOptions.toolVersion;
String get relCanonicalPrefix => htmlOptions.relCanonicalPrefix;
bool get useBaseHref => htmlOptions.useBaseHref;

String _layoutTitle(String name, String kind, bool isDeprecated) =>
_renderHelper.composeLayoutTitle(name, kind, isDeprecated);
Expand Down Expand Up @@ -67,9 +69,9 @@ class PackageTemplateData extends TemplateData<Package> {
bool get hasHomepage => package.hasHomepage;
String get homepage => package.homepage;

/// `null` for packages because they are at the root – not needed
/// empty for packages because they are at the root – not needed
@override
String get htmlBase => null;
String get htmlBase => '';
}

class CategoryTemplateData extends TemplateData<Category> {
Expand All @@ -83,7 +85,7 @@ class CategoryTemplateData extends TemplateData<Category> {
String get title => '${category.name} ${category.kind} - Dart API';

@override
String get htmlBase => '..';
String get htmlBase => '../';

@override
String get layoutTitle => _layoutTitle(category.name, category.kind, false);
Expand All @@ -109,7 +111,7 @@ class LibraryTemplateData extends TemplateData<Library> {
@override
String get title => '${library.name} library - Dart API';
@override
String get htmlBase => '..';
String get htmlBase => '../';
@override
String get metaDescription =>
'${library.name} library API docs, for the Dart programming language.';
Expand Down Expand Up @@ -164,7 +166,7 @@ class ClassTemplateData<T extends Class> extends TemplateData<T> {
@override
List get navLinks => [packageGraph.defaultPackage, library];
@override
String get htmlBase => '..';
String get htmlBase => '../';

Class get objectType {
if (_objectType != null) {
Expand Down Expand Up @@ -207,7 +209,7 @@ class ExtensionTemplateData<T extends Extension> extends TemplateData<T> {
@override
List get navLinks => [packageGraph.defaultPackage, library];
@override
String get htmlBase => '..';
String get htmlBase => '../';
}

class ConstructorTemplateData extends TemplateData<Constructor> {
Expand Down Expand Up @@ -235,7 +237,7 @@ class ConstructorTemplateData extends TemplateData<Constructor> {
List get navLinksWithGenerics => [clazz];
@override
@override
String get htmlBase => '../..';
String get htmlBase => '../../';
@override
String get title => '${constructor.name} constructor - ${clazz.name} class - '
'${library.name} library - Dart API';
Expand Down Expand Up @@ -279,7 +281,7 @@ class FunctionTemplateData extends TemplateData<ModelFunction> {
@override
List get navLinks => [packageGraph.defaultPackage, library];
@override
String get htmlBase => '..';
String get htmlBase => '../';
}

class MethodTemplateData extends TemplateData<Method> {
Expand Down Expand Up @@ -317,7 +319,7 @@ class MethodTemplateData extends TemplateData<Method> {
@override
List get navLinksWithGenerics => [container];
@override
String get htmlBase => '../..';
String get htmlBase => '../../';
}

class PropertyTemplateData extends TemplateData<Field> {
Expand Down Expand Up @@ -356,7 +358,7 @@ class PropertyTemplateData extends TemplateData<Field> {
@override
List get navLinksWithGenerics => [container];
@override
String get htmlBase => '../..';
String get htmlBase => '../../';

String get type => 'property';
}
Expand Down Expand Up @@ -400,7 +402,7 @@ class TypedefTemplateData extends TemplateData<Typedef> {
@override
List get navLinks => [packageGraph.defaultPackage, library];
@override
String get htmlBase => '..';
String get htmlBase => '../';
}

class TopLevelPropertyTemplateData extends TemplateData<TopLevelVariable> {
Expand Down Expand Up @@ -431,7 +433,7 @@ class TopLevelPropertyTemplateData extends TemplateData<TopLevelVariable> {
@override
List get navLinks => [packageGraph.defaultPackage, library];
@override
String get htmlBase => '..';
String get htmlBase => '../';

String get _type => 'property';
}
Expand Down
13 changes: 12 additions & 1 deletion lib/src/model/package.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ import 'package:pub_semver/pub_semver.dart';

final RegExp substituteNameVersion = RegExp(r'%([bnv])%');

// All hrefs are emitted as relative paths from the output root. We are unable
// to compute them from the page we are generating, and many properties computed
// using hrefs are memoized anyway. To build complete relative hrefs, we emit
// the href with this placeholder, and then replace it with the current page's
// base href afterwards.
// See https://github.com/dart-lang/dartdoc/issues/2090 for further context.
// TODO: Find an approach that doesn't require doing this.
// Unlikely to be mistaken for an identifier, html tag, or something else that
// might reasonably exist normally.
final String HTMLBASE_PLACEHOLDER = '\%\%__HTMLBASE_dartdoc_internal__\%\%';

/// A [LibraryContainer] that contains [Library] objects related to a particular
/// package.
class Package extends LibraryContainer
Expand Down Expand Up @@ -206,7 +217,7 @@ class Package extends LibraryContainer
});
if (!_baseHref.endsWith('/')) _baseHref = '${_baseHref}/';
} else {
_baseHref = '';
_baseHref = config.useBaseHref ? '' : HTMLBASE_PLACEHOLDER;
}
}
return _baseHref;
Expand Down
9 changes: 5 additions & 4 deletions lib/templates/_footer.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
<!-- footer-text placeholder -->
</footer>

{{! TODO(jdkoren): unwrap ^useBaseHref sections when the option is removed.}}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
<script src="static-assets/typeahead.bundle.min.js"></script>
<script src="static-assets/highlight.pack.js"></script>
<script src="static-assets/URI.js"></script>
<script src="static-assets/script.js"></script>
<script src="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/typeahead.bundle.min.js"></script>
<script src="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/highlight.pack.js"></script>
<script src="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/URI.js"></script>
<script src="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/script.js"></script>
<!-- footer placeholder -->

</body>
Expand Down
10 changes: 7 additions & 3 deletions lib/templates/_head.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,20 @@
{{ #relCanonicalPrefix }}
<link rel="canonical" href="{{{relCanonicalPrefix}}}/{{{self.href}}}">
{{ /relCanonicalPrefix}}

{{#useBaseHref}}{{! TODO(jdkoren): remove when the useBaseHref option is removed.}}
{{#htmlBase}}
<!-- required because all the links are pseudo-absolute -->
<base href="{{{htmlBase}}}">
{{/htmlBase}}
{{/useBaseHref}}

{{! TODO(jdkoren): unwrap ^useBaseHref sections when the option is removed.}}
<link href="https://fonts.googleapis.com/css?family=Source+Code+Pro:500,400i,400,300|Source+Sans+Pro:400,300,700" rel="stylesheet">
<link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">
<link rel="stylesheet" href="static-assets/github.css">
<link rel="stylesheet" href="static-assets/styles.css">
<link rel="icon" href="static-assets/favicon.png">
<link rel="stylesheet" href="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/github.css">
<link rel="stylesheet" href="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/styles.css">
<link rel="icon" href="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/favicon.png">
<!-- header placeholder -->
</head>

Expand Down
6 changes: 4 additions & 2 deletions test/model_special_cases_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,8 @@ void main() {
hashCode.inheritance.any((c) => c.name == 'Interceptor'), isTrue);
// If EventTarget really does start implementing hashCode, this will
// fail.
expect(hashCode.href, equals('dart-core/Object/hashCode.html'));
expect(hashCode.href,
equals('${HTMLBASE_PLACEHOLDER}dart-core/Object/hashCode.html'));
expect(
hashCode.canonicalEnclosingContainer, equals(objectModelElement));
expect(
Expand Down Expand Up @@ -387,7 +388,8 @@ void main() {
test('sdk library have formatted names', () {
expect(dartAsyncLib.name, 'dart:async');
expect(dartAsyncLib.dirName, 'dart-async');
expect(dartAsyncLib.href, 'dart-async/dart-async-library.html');
expect(dartAsyncLib.href,
'${HTMLBASE_PLACEHOLDER}dart-async/dart-async-library.html');
});
});

Expand Down
Loading