Code review fixes

- Add a CLI flag (hidden) to use the legacy behavior
- Change placeholder string
- Detailed comment regarding usage of the placeholder

Author: jdkoren

lib/src/dartdoc_options.dart

@@ -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
@@ -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.
   ]

lib/src/html/html_generator.dart

@@ -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';
 }
 
@@ -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(

lib/src/html/html_generator_instance.dart

@@ -76,7 +76,9 @@ class HtmlGeneratorInstance {
     });
 
     String json = encoder.convert(indexItems);
-    json = json.replaceAll(HTMLBASE_PLACEHOLDER, '');
+    if (!_options.useBaseHref) {
+      json = json.replaceAll(HTMLBASE_PLACEHOLDER, '');
+    }
     _writer(path.join('categories.json'), '${json}\n');
   }
 
@@ -119,7 +121,9 @@ class HtmlGeneratorInstance {
     });
 
     String json = encoder.convert(indexItems);
-    json = json.replaceAll(HTMLBASE_PLACEHOLDER, '');
+    if (!_options.useBaseHref) {
+      json = json.replaceAll(HTMLBASE_PLACEHOLDER, '');
+    }
     _writer(path.join('index.json'), '${json}\n');
   }
 

lib/src/html/template_data.dart

@@ -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> {
@@ -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);

lib/src/model/package.dart

@@ -14,9 +14,16 @@ 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\%\%';
+final String HTMLBASE_PLACEHOLDER = '\%\%__HTMLBASE_dartdoc_internal__\%\%';
 
 /// A [LibraryContainer] that contains [Library] objects related to a particular
 /// package.
@@ -210,7 +217,7 @@ class Package extends LibraryContainer
         });
         if (!_baseHref.endsWith('/')) _baseHref = '${_baseHref}/';
       } else {
-        _baseHref = HTMLBASE_PLACEHOLDER;
+        _baseHref = config.useBaseHref ? '' : HTMLBASE_PLACEHOLDER;
       }
     }
     return _baseHref;

lib/templates/_footer.html

@@ -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="%%HTMLBASE%%static-assets/typeahead.bundle.min.js"></script>
-<script src="%%HTMLBASE%%static-assets/highlight.pack.js"></script>
-<script src="%%HTMLBASE%%static-assets/URI.js"></script>
-<script src="%%HTMLBASE%%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>

lib/templates/_head.html

@@ -13,11 +13,19 @@
   <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="%%HTMLBASE%%static-assets/github.css">
-  <link rel="stylesheet" href="%%HTMLBASE%%static-assets/styles.css">
-  <link rel="icon" href="%%HTMLBASE%%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>