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

webview: Get HTML out of "quirks mode" #3098

Open
3 tasks
gnprice opened this issue Nov 1, 2018 · 3 comments
Open
3 tasks

webview: Get HTML out of "quirks mode" #3098

gnprice opened this issue Nov 1, 2018 · 3 comments
Labels
a-layout a-message list

Comments

@gnprice
Copy link
Member

gnprice commented Nov 1, 2018 •
edited
Loading

(Discovered this while working on #3040 .)

Our HTML is actually invoking "quirks mode". We should add a <!DOCTYPE html>.

As explained in the MDN article Quirks Mode and Standards Mode:

In quirks mode, layout emulates nonstandard behavior in Navigator 4 and Internet Explorer 5. This is essential in order to support websites that were built before the widespread adoption of web standards. In full standards mode, the behavior is (hopefully) the behavior described by the HTML and CSS specifications.

So we should move to "full standards mode". (The specs call this no-quirks mode.)

Here's the bare skeleton of a start on this:

diff --git src/webview/html/html.js src/webview/html/html.js
index a77947969..68df99bbf 100644
--- src/webview/html/html.js
+++ src/webview/html/html.js
@@ -13,6 +13,8 @@ type InitOptionsType = {
 };
 
 export default (content: string, theme: ThemeType, initOptions: InitOptionsType) => template`
+<!DOCTYPE html>
+<html>
 $!${script(initOptions.anchor, initOptions.auth)}
 $!${css(theme, initOptions.highlightUnreadMessages)}
 
@@ -20,4 +22,5 @@ $!${css(theme, initOptions.highlightUnreadMessages)}
 <body style="overflow-x: hidden;">
 $!${htmlBody(content, initOptions.showMessagePlaceholders)}
 </body>
+</html>
 `;

The actual work of resolving this issue is what comes after that 😉 :

  • Skim through MDN's list of differences looking for things that might need adjustment for us.
  • Check our scrolling behavior -- that's one area where quirks mode changes things. Fix what needs fixing.
    • From that MDN list: > The scrollLeft, scrollTop, scrollWidth, and scrollHeight properties are relative to BODY in quirks mode (instead of HTML)
    • Also documentBody.clientHeight changes.
  • Test the app in other ways, especially areas identified in step 1. Fix what needs fixing.
@gnprice gnprice mentioned this issue Nov 1, 2018
Merged
@AB261
Copy link
Collaborator

AB261 commented Mar 30, 2019

@zulipbot claim
@gnprice I would like to work on this and will look into the changes needed to move into full standards mode.

@gnprice gnprice mentioned this issue Nov 12, 2019
Closed
@gnprice
Copy link
Member Author

gnprice commented Nov 12, 2019

We tested this out over on #3679 (by @ray-kraesig) -- the branch added a doctype, and I nodded right past that (ah good, yes, we should get out of quirks mode) until I spotted some funny behavior and discovered to my surprise that it was that change that had introduced it 🙂 . Then I remembered we'd looked at this before, i.e. in this thread.

The symptom I found if you just add a doctype and html tags -- exactly like the diff quoted above -- is:

  • Say you're in a narrow with a long history, scrolled a ways back.
  • A new message comes in.
  • The message list will scroll all the way to the bottom.

Once we add KaTeX (that's #3679), it starts complaining (to the console, IIUC) about being in quirks mode. So that adds some motivation to deal with this.

@rk-for-zulip
Copy link
Contributor

(to the console, IIUC)

(For the record: you do.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a-layout a-message list
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gnprice @AB261 @rk-for-zulip