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

Private/gokay/separate js #9442

Merged
merged 13 commits into from
Jul 29, 2024
Merged

Private/gokay/separate js #9442

merged 13 commits into from
Jul 29, 2024

Conversation

gokaysatir
Copy link
Contributor

  • Resolves: #
  • Target version: master

Summary

TODO

  • ...

Checklist

  • Code is properly formatted
  • All commits have Change-Id
  • I have run tests with make check
  • I have issued make run and manually verified that everything looks okay
  • Documentation (manuals or wiki) has been updated or is not required

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 5, 2024
View reviewed changes
browser/js/global.js Fixed Show fixed Hide fixed
browser/js/global.js Dismissed Show dismissed Hide dismissed
browser/js/global.js Dismissed Show dismissed Hide dismissed
browser/js/global.js Fixed Show fixed Hide fixed
browser/js/global.js Dismissed Show dismissed Hide dismissed
@timar timar force-pushed the private/gokay/separate-js branch from 34fab9d to ce85976 Compare July 6, 2024 11:00
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 8, 2024
View reviewed changes
browser/js/global.js Dismissed Show dismissed Hide dismissed
browser/js/global.js Dismissed Show dismissed Hide dismissed
browser/js/global.js Dismissed Show dismissed Hide dismissed
browser/js/global.js Dismissed Show dismissed Hide dismissed
browser/js/global.js Dismissed Show dismissed Hide dismissed
browser/js/global.js Dismissed Show dismissed Hide dismissed
@gokaysatir gokaysatir force-pushed the private/gokay/separate-js branch 2 times, most recently from 7ed47be to d0e9689 Compare July 8, 2024 14:51
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 8, 2024
View reviewed changes
browser/js/global.js Fixed Show fixed Hide fixed
@gokaysatir gokaysatir force-pushed the private/gokay/separate-js branch 4 times, most recently from 386e493 to d1963ad Compare July 8, 2024 18:58
@gokaysatir gokaysatir requested a review from eszkadev July 8, 2024 18:58
@gokaysatir gokaysatir force-pushed the private/gokay/separate-js branch 4 times, most recently from df9ea6a to 47aea73 Compare July 9, 2024 14:27
mmeeks
mmeeks reviewed Jul 10, 2024
View reviewed changes
Copy link
Contributor

@mmeeks mmeeks left a comment

Choose a reason for hiding this comment

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

Looks rather good to me - thanks Gokay ! =)

Having said that - I think it needs to go in early in the next 'red' release; and we should give guidance to the requester of the timeline there.

@gokaysatir gokaysatir force-pushed the private/gokay/separate-js branch 3 times, most recently from f1db862 to f52c0a6 Compare July 19, 2024 12:12
@mmeeks mmeeks force-pushed the private/gokay/separate-js branch from f52c0a6 to 440f32e Compare July 27, 2024 11:26
@mmeeks
Copy link
Contributor

mmeeks commented Jul 27, 2024

Split / squashed and merged chunks this together - hopefully it makes it easier to review =) fewer 'Fixup' commits after the fact for bits of parent commits.

@gokaysatir @mmeeks
Move definitions and JS code into global.js from cool.html.m4
530187b 
Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
Change-Id: I3155e799530d4c307569e88612a13570d0913e03
@gokaysatir @mmeeks
Use http / https instead of wss / ws for XMLHttp requests.
a46e046 
Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
Change-Id: I9695ee748b43702d2295fec5c94c606d8d460d5f
@gokaysatir @mmeeks
Had to add some forced-rules for device-mobile css.
be55dc5 
Remove extra changes for ws requests.

For some reason, the priority of device-mobile.css -> cool.css and menubar.css has changed.
This causes same rules to overwrite the others.

Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
Change-Id: I87e568a8115c6cf19233cc0ff01c9521d7e5d3f1
@mmeeks mmeeks mentioned this pull request Jul 29, 2024
Open
@gokaysatir
Copy link
Contributor Author

Thanks for the review :)

@thebearon thebearon mentioned this pull request Jul 31, 2024
Merged
Minion3665 added a commit that referenced this pull request Jul 31, 2024
@Minion3665
fix(iOS): Allow document loading
c52194f 
Commit 8f0f931 from #9442 broke both mobile apps in several ways...

In addition to the fixes which android needed (see #9669), there were 2
things broken in the iOS app only. Here they are:
- global.js wasn't included in the app resources (fixed by Patrick Luby
  in #9673)
- A garbled m4 macro caused some css to fail in loading, and -- my
  current understanding is -- as we use things like the page height in
  rendering this caused us to have severe issues when running our
  JavaScript. That's fixed in this commit
Minion3665 added a commit that referenced this pull request Jul 31, 2024
@Minion3665
fix(iOS): Allow document loading
3a1f521 
Commit 8f0f931 from #9442 broke both mobile apps in several ways...

In addition to the fixes which android needed (see #9669), there were 2
things broken in the iOS app only. Here they are:
- global.js wasn't included in the app resources (fixed by Patrick Luby
  in #9673)
- A garbled m4 macro caused some css to fail in loading, and -- my
  current understanding is -- as we use things like the page height in
  rendering this caused us to have severe issues when running our
  JavaScript. That's fixed in this commit

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: Ie8580567bf8426fb9ae2f159acf5994238614795
@Minion3665 Minion3665 mentioned this pull request Jul 31, 2024
Merged
Minion3665 added a commit that referenced this pull request Jul 31, 2024
@Minion3665
fix(iOS): Allow document loading
d16f455 
Commit 8f0f931 from #9442 broke both mobile apps in several ways...

In addition to the fixes which android needed (see #9669), there were 2
things broken in the iOS app only. Here they are:
- global.js wasn't included in the app resources (fixed by Patrick Luby
  in #9673)
- A garbled m4 macro caused some css to fail in loading, and -- my
  current understanding is -- as we use things like the page height in
  rendering this caused us to have severe issues when running our
  JavaScript. That's fixed in this commit

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: Ie8580567bf8426fb9ae2f159acf5994238614795
Minion3665 added a commit that referenced this pull request Aug 1, 2024
@Minion3665
fix(iOS): Allow document loading
8192d59 
Commit 8f0f931 from #9442 broke both mobile apps in several ways...

In addition to the fixes which android needed (see #9669), there were 2
things broken in the iOS app only. Here they are:
- global.js wasn't included in the app resources (fixed by Patrick Luby
  in #9673)
- A garbled m4 macro caused some css to fail in loading, and -- my
  current understanding is -- as we use things like the page height in
  rendering this caused us to have severe issues when running our
  JavaScript. That's fixed in this commit

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: Ie8580567bf8426fb9ae2f159acf5994238614795
@juliushaertl juliushaertl mentioned this pull request Aug 7, 2024
Closed
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): Surface real information in "about"
4083026 
Previously we exposed "%WOPI_HOST_ID%" and "%PROXY_PREFIX_ENABLED%",
despoenite those variables being undefined for mobile. Additionally, we
incorrectly displayed the app name as MOBILEAPPNAME.

These appear to be regressions from #9442
@Minion3665 Minion3665 mentioned this pull request Aug 23, 2024
Merged
6 tasks
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): Surface real information in "about"
b53e7ed 
Previously we exposed "%WOPI_HOST_ID%" and "%PROXY_PREFIX_ENABLED%",
despoenite those variables being undefined for mobile. Additionally, we
incorrectly displayed the app name as MOBILEAPPNAME.

These appear to be regressions from #9442
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): Surface real information in "about"
2c58a7e 
Previously we exposed WOPI_HOST_ID and PROXY_PREFIX_ENABLED,
despite those variables being undefined for mobile. Additionally, we
incorrectly displayed the app name as MOBILEAPPNAME.

These appear to be regressions from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I2dde8cee90d77e181f31d7e579a222a7fdb86559
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): Surface real information in "about"
e953674 
Previously we exposed WOPI_HOST_ID and PROXY_PREFIX_ENABLED,
despite those variables being undefined for mobile. Additionally, we
incorrectly displayed the app name as MOBILEAPPNAME.

These appear to be regressions from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I2dde8cee90d77e181f31d7e579a222a7fdb86559
@Minion3665 Minion3665 mentioned this pull request Aug 23, 2024
Merged
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): reorder stylesheets
20179dd 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
@Minion3665 Minion3665 mentioned this pull request Aug 23, 2024
Merged
6 tasks
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): reorder stylesheets
2c82db3 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
@Minion3665 Minion3665 mentioned this pull request Aug 23, 2024
Merged
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): reorder stylesheets
6cafef2 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): Surface real information in "about"
faeb141 
Previously we exposed WOPI_HOST_ID and PROXY_PREFIX_ENABLED,
despite those variables being undefined for mobile. Additionally, we
incorrectly displayed the app name as MOBILEAPPNAME.

These appear to be regressions from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I2dde8cee90d77e181f31d7e579a222a7fdb86559
timar pushed a commit that referenced this pull request Aug 23, 2024
@Minion3665 @timar
fix(mobile): Surface real information in "about"
b530fc8 
Previously we exposed WOPI_HOST_ID and PROXY_PREFIX_ENABLED,
despite those variables being undefined for mobile. Additionally, we
incorrectly displayed the app name as MOBILEAPPNAME.

These appear to be regressions from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I2dde8cee90d77e181f31d7e579a222a7fdb86559
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): reorder stylesheets
7374cd4 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): reorder stylesheets
76c255e 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): reorder stylesheets
7d49b81 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
eszkadev pushed a commit that referenced this pull request Aug 23, 2024
@Minion3665 @eszkadev
fix(mobile): Surface real information in "about"
143e6c3 
Previously we exposed WOPI_HOST_ID and PROXY_PREFIX_ENABLED,
despite those variables being undefined for mobile. Additionally, we
incorrectly displayed the app name as MOBILEAPPNAME.

These appear to be regressions from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I2dde8cee90d77e181f31d7e579a222a7fdb86559
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): reorder stylesheets
ff31007 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
Minion3665 added a commit that referenced this pull request Aug 23, 2024
@Minion3665
fix(mobile): reorder stylesheets
6f235f6 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from #9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
andreasma pushed a commit to freeonlineoffice/online that referenced this pull request Aug 24, 2024
@Minion3665 @andreasma
fix(mobile): Surface real information in "about"
9de6228 
Previously we exposed WOPI_HOST_ID and PROXY_PREFIX_ENABLED,
despite those variables being undefined for mobile. Additionally, we
incorrectly displayed the app name as MOBILEAPPNAME.

These appear to be regressions from CollaboraOnline/online#9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I2dde8cee90d77e181f31d7e579a222a7fdb86559
andreasma pushed a commit to freeonlineoffice/online that referenced this pull request Aug 24, 2024
@Minion3665 @andreasma
fix(mobile): reorder stylesheets
ba2f3f5 
As we insert other stylesheets after the head (and therefore later than
stylesheets in the head), they take precedence over stylesheets in the
head.

Therefore, we can't insert our device and branding stylesheets in the
end of the <head>. We need to insert them after the latest other
stylesheet.

This appears to a regression from CollaboraOnline/online#9442

Signed-off-by: Skyler Grey <skyler.grey@collabora.com>
Change-Id: I829c0dc1ad95a28650845d39aad9f975692319b8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmeeks mmeeks mmeeks left review comments

@meven meven meven left review comments

@eszkadev eszkadev Awaiting requested review from eszkadev

@mcecchetti mcecchetti Awaiting requested review from mcecchetti

Assignees
No one assigned
Labels
CODE "red"
Projects
Collabora Online
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gokaysatir @mmeeks @meven @timar