Skip to content

Fix logical error of filling fullscreenElements (3.12) #145

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 1 commit into from
Feb 25, 2019

Conversation

ChangWanHong
Copy link
Contributor

@ChangWanHong ChangWanHong commented Feb 22, 2019

Closes: #144


Preview | Diff

@ChangWanHong
Copy link
Contributor Author

CI tool emits similar log with my local "make deploy":

$ make deploy 
curl --remote-name --fail https://resources.whatwg.org/build/deploy.sh
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  8555  100  8555    0     0   5684      0  0:00:01  0:00:01 --:--:--  5688
bash ./deploy.sh
Running a local deploy into fullscreen.spec.whatwg.org directory

Running deploy for commit: 23899b4442ba622c0024ce4f5c6f4b02c24f7fe2

Linting the source:
All good

Starting commit snapshot...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 31022    0   795  100 30227    383  14562  0:00:02  0:00:02 --:--:-- 14567
Error running preprocessor, returned code: 1.
LINK ERROR: Multiple possible 'browsing context' dfn refs for '/'.
Arbitrarily chose https://html.spec.whatwg.org/multipage/browsers.html#browsing-context
The following refs show up multiple times in their spec, in a way that Bikeshed can't distinguish between. Either create a manual link, or ask the spec maintainer to add disambiguating attributes (usually a for='' attribute to all of them).
spec:html; type:dfn; for:/; text:browsing context
  https://html.spec.whatwg.org/multipage/browsers.html#browsing-context
  https://html.spec.whatwg.org/multipage/window-object.html#window-bc
<a data-link-for="/" data-link-type="dfn" data-lt="browsing context">browsing context</a>
 ✘  Did not generate, due to fatal errorsMakefile:8: recipe for target 'deploy' failed
make: *** [deploy] Error 22

It's not from my patch, but should I resolve LINK ERROR first?

@annevk
Copy link
Member

annevk commented Feb 22, 2019

@ChangWanHong please don't worry about it. I'll try to fix this in the HTML Standard.

annevk added a commit to whatwg/html that referenced this pull request Feb 22, 2019
@annevk
Copy link
Member

annevk commented Feb 22, 2019

@ChangWanHong one thing you will need to do is sign https://participate.whatwg.org/agreement. Sorry for not making that clear earlier.

Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

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

Thanks @ChangWanHong!

This error was introduced by me in #92, where I changed from first+prepend to first+append.

domenic pushed a commit to whatwg/html that referenced this pull request Feb 22, 2019
@ChangWanHong
Copy link
Contributor Author

@annevk I signed as individual! Thanks for your help.

@foolip Thanks for your clear review! I hope this would be enough to merge.

@foolip
Copy link
Member

foolip commented Feb 25, 2019

Thanks @ChangWanHong!

@annevk I'll restart the Travis build, it should be fixed now right?

@ChangWanHong
Copy link
Contributor Author

Hmm. It seems I'm not verified yet.

From signing page (after I signed),

...
Note that for now your submission is marked as unverified. The first time you submit a pull request to a standard, the editor will be responsible for verifying that your submission is accurate and representative. After that, future pull requests to the workstreams you chose to participate in will be automatically marked as approved.
...

@foolip Could you verify me please :)

@ChangWanHong
Copy link
Contributor Author

@foolip One more thing, It is not relevant on this.. but I found some issue on your chromium code. PTAL: https://bugs.chromium.org/p/chromium/issues/detail?id=934150 . I couldn't CC'ing you, may be I don't have permission.

@foolip
Copy link
Member

foolip commented Feb 25, 2019

@ChangWanHong verified, and merging now. Thanks!

Commented on https://bugs.chromium.org/p/chromium/issues/detail?id=934150.

@foolip foolip merged commit d2e9d66 into whatwg:master Feb 25, 2019
@ChangWanHong ChangWanHong deleted the issue-144 branch February 26, 2019 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants