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

update using latest bikeshed version (tabakins/bikeshed#a3b64e5) #471

Closed
wants to merge 7 commits into from

Conversation

gkatsev
Copy link
Collaborator

@gkatsev gkatsev commented Jul 16, 2019


💥 Error: 400 Bad Request 💥

PR Preview failed to build. (Last tried on Dec 10, 2020, 12:15 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 Related URL

Error running preprocessor, returned code: 2.
WARNING: The following <var>s were only used once in the document:
  'stylesheets', in algorithm 'WebVTT parser algorithm'
  'input', in algorithm 'WebVTT region settings parsing'
  'input', in algorithm 'WebVTT region objects'
  'regions', in algorithm 'collect WebVTT cue timings and settings'
  'input', in algorithm 'parse the WebVTT cue settings'
  'regions', in algorithm 'parse the WebVTT cue settings'
  'language', in algorithm 'WebVTT cue text parsing rules'
  'cue', in algorithm 'WebVTT rules for extracting the chapter
title'
  'cue', in algorithm 'WebVTT rules for extracting the chapter title'
  'language', in algorithm 'Processing model'
  'language', in algorithm 'rules for updating the display of WebVTT text tracks'
  'nodes', in algorithm 'Processing cue settings'
  'left', in algorithm 'apply WebVTT cue settings'
  'top', in algorithm 'apply WebVTT cue settings'
  'max dimension', in algorithm 'apply WebVTT cue settings'
  'video', in algorithm 'Obtaining CSS boxes'
  'startTime', in algorithm 'VTTCue construction'
  'endTime', in algorithm 'VTTCue construction'
  'text', in algorithm 'VTTCue construction'
If these are not typos, please add an ignore='' attribute to the <var>.
FATAL ERROR: The [Constructor] extended attribute (on VTTCue) is deprecated, please switch to a constructor() method.
 ✘  Did not generate, due to fatal errors

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@gkatsev
Copy link
Collaborator Author

gkatsev commented Jul 16, 2019

Looking at the commit mentioned in the README (754f13e) about the manual fixup, a lot of the changes in that commit don't exist anymore.

I do see some ids change from something like ref-for-webvtt-cue-box-5 to ref-for-webvtt-cue-box④, I could revert those if we wanted to keep those the same. Might be better to just accept the changes and move on.

@silviapfeiffer
Copy link
Member

Id changes don't matter, as long as the document comes out consistent.

@gkatsev
Copy link
Collaborator Author

gkatsev commented Jul 16, 2019

The subsequent commits address some of bikeshed's warnings around auto-linking. We weren't properly linkifying some css selectors. I can move them to another PR also, though.

Copy link
Member

@silviapfeiffer silviapfeiffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WFM

@gkatsev
Copy link
Collaborator Author

gkatsev commented Jul 29, 2019

@plehegar do you have a moment to look through and see if there are any bad links or links that need to be updated? Thanks!

@himorin
Copy link
Contributor

himorin commented Jul 30, 2019

proposal: add two lines for defs

@@ -61,6 +61,8 @@ urlPrefix: https://html.spec.whatwg.org/multipage/
             text: entry settings object
         urlPrefix: syntax.html
             text: character references; url: #syntax-charref
+            text: additional allowed character
+            text: consume a character reference
     type: element-attr
         urlPrefix: dom.html
             text: title; url: #attr-title

I have no idea on 'min-height' and 'max-height' with css22...

LINK ERROR: No 'property' refs found for 'min-height' with spec 'css22'.
LINK ERROR: No 'property' refs found for 'max-height' with spec 'css22'.

@xfq
Copy link
Member

xfq commented Jul 30, 2019

I think adding "link defaults" should fix the issue. As an example:

https://github.com/w3c/csswg-drafts/blob/09101423ab89bfeff5f95a979fb24acc1a6ccf5c/css-sizing-3/Overview.bs#L22-L23

@xfq
Copy link
Member

xfq commented Jul 30, 2019

(We generally use css2 instead of css22 for now.)

@gkatsev
Copy link
Collaborator Author

gkatsev commented Jul 30, 2019

We have the link defaults (https://github.com/w3c/webvtt/blob/gh-pages/index.bs#L82-L83) but somehow the css22 isn't specific enough. bikeshed suggested using css23, I think when I had it show suggestions but didn't seem right. I'll try just using css2.

@himorin
Copy link
Contributor

himorin commented Aug 2, 2019

I should ask to bikehsed (w/ filing an issue), but in current configuration bikeshed/specdata/readonly/specs.jsonpwd has a block of:

  "css2": {
    "abstract": null, 
    "current_url": "https://drafts.csswg.org/css2/", 
    "description": "Cascading Style Sheets Level 2 Revision 1", 
    "domain": "css", 
    "level": 1, 
    "shortname": "css2", 
    "snapshot_url": "https://www.w3.org/TR/CSS21/", 
    "status": null, 
    "title": "CSS 2.1", 
    "vshortname": "css2", 
    "work_status": null, 
    "working_group": "csswg"
  }, 

whose current_url points to CSS2.2 but titled as CSS 2.1. Also, all definitions in bikeshed/specdata/headings/headings-css2.json says spec to CSS 2.1 like

  "/box#bidi-box-model": {
    "current": {
      "number": "8.6", 
      "spec": "CSS 2.1", 
      "text": "The box model for inline elements in bidirectional context", 
      "url": "https://drafts.csswg.org/css2/box.html#bidi-box-model"
    }, 
    "snapshot": {
      "number": "8.6", 
      "spec": "CSS 2.1", 
      "text": "The box model for inline elements in bidirectional context", 
      "url": "https://www.w3.org/TR/CSS21/box.html#bidi-box-model"
    }
  }, 

so, it seems CSS 2.2 is not taken as target, and all are set to point CSS 2.1.

as @xfq pointed if css2 is one generally used, should we keep targeting to use css22 for reference or switch to css2 (2.1)?

@silviapfeiffer
Copy link
Member

It's actually quite simple to make a pull request for Bikeshed's library of references.

@himorin
Copy link
Contributor

himorin commented Aug 5, 2019

trying to point where to change for making PR...

adding following to anchors block could work, but three (height, line-height, width) was marked as CSS2 in terms defined by reference.

urlPrefix: https://drafts.csswg.org/css2/                                       
    type: property                                                              
        urlPrefix: visudet.html                                                 
            text:min-height; url: #propdef-min-height                           
            text:max-height; url: #propdef-max-height                           
            text:height; url: #propdef-height                                   
            text:width; url: #propdef-width                                     
            text:line-height; url: #propdef-line-height                         
        urlPrefix: visufx.html                                                  
            text:visibility; url: #propdef-visibility

@himorin
Copy link
Contributor

himorin commented Aug 5, 2019

https://api.specref.org/bibrefs has CSS2.2 (that's why we can generate [CSS22] reference, I suppose), also all of spec-data/headings/headings-css2.json points CSS 2.1... It seems we need to update whole external spec data files using CSS 2.2 to make all reference links in our spec to target CSS 2.2 (correct??). There is CSS 2.2 FPWD, and I am not sure why it is not enabled in... (let me search a bit more..)

@gkatsev
Copy link
Collaborator Author

gkatsev commented Aug 7, 2019

The urlPrefix change for the css2 works, but then min-height, max-height, and visibility are removed for the "[CSS2] defines the following terms:" part of the terms defined by reference section.

It's also weird because if you remove the two min-height and max-height lines from the link-defaults section, bikeshed itself suggests we add the lines as we currently have. I wonder if it's related to speced/bikeshed#1229.

@himorin
Copy link
Contributor

himorin commented Aug 8, 2019

The urlPrefix change for the css2 works, but then min-height, max-height, and visibility are removed for the "[CSS2] defines the following terms:" part of the terms defined by reference section.

For this, how about to add 'spec: css22;' right before the first urlPrefix in the block?

@himorin himorin changed the base branch from gh-pages to main December 10, 2020 12:14
@gkatsev
Copy link
Collaborator Author

gkatsev commented Dec 22, 2022

I created #509 with some more updates on top of this. Once that's in, I'll rebase my outstanding PRs since they're currently blocked from being merged due to the index.html conflict, like this PR.

@gkatsev gkatsev closed this Dec 22, 2022
@gkatsev gkatsev deleted the bikeshed-updates branch December 22, 2022 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants