Skip to content

/uBlock

Loading…

Safari: more compatible build, better popover sizing #477

Merged
merged 5 commits into from
+52 −52

Labels

None yet

Milestone

No milestone

Assignee

No one assigned

3 participants

@chrisaljoudi @Deathamns @gorhill
@chrisaljoudi
Owner
chrisaljoudi commented

No description provided.

@chrisaljoudi
Owner
chrisaljoudi commented

@gorhill hmm, interesting. I'll have to look at the Firefox code more.

I've added vadi-popup.js for Safari and moved that popup-specific stuff out of vapi-common.js. Also, the mini library for size-change detection is now non-minified.

@gorhill gorhill merged commit 87ccd82 into chrisaljoudi:master
@Deathamns
Deathamns commented

@chrisaljoudi On the Firefox branch many changes were added to the Safari code too, which are not pushed to the master here, but they're more up to date. For example the compatibility with Python2.

The idea behind using mutation observers was to achieve transparent behavior, since no extra elements will be added to the popup. As a result it removes the chance of interfering with the main application in any way.

@gorhill
gorhill commented

no extra elements will be added to the popup

I realized this afterward reading the code, and really I am not sure what to think, so I leave it to the platform-specific code to decide. Your mutation observer is almost a noop, so I don't really fear an overhead problem -- despite the popup UI removing/adding a node on mouse move.

@chrisaljoudi
Owner
chrisaljoudi commented

@gorhill @Deathamns sure, I get the logic.

It's up to you, I guess. Sure, the current way of listening adds two invisible children (which should also have no effect on the layout) — but it only fires on size changes (the mutation observer will fire on modifications to children that may not have effect on size).

In any case, it's up to you.

It's worth mentioning that if we do go back to mutation observers, we should probably still keep the changes to the size calculation from this pull (not hard coding the magic constants that are the initial size).

@gorhill
gorhill commented

@chrisaljoudi as said I really don't have an opinion. On one side there is the extra overhead of mutation observer firing a lot compare to scroll event, on the other there is tampering with the popup UI which could be a problem in the future if adding some CSS rules which affect the added nodes. I just can't decide what is best. I will leave it to you two to decide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 12, 2015

  1. More compatible build -- use codecs module

    committed
    c5e5ad6

  2. Better Safari popover sizing, no mutation events

    committed
    0d03499

  3. Merge branch 'master' of github.com:gorhill/uBlock

    committed
    b57d042

  4. Abstract out platform-specific popup UI JavaScript

    committed
    0bd2458

  5. Merge branch 'master' of github.com:gorhill/uBlock

    committed
    c32ea31
This page is out of date. Refresh to see the latest.
Unified Split
Showing with 52 additions and 52 deletions.
  1. +0 −4 platform/safari/Info.plist
  2. +0 −42 platform/safari/vapi-common.js
  3. +45 −0 platform/safari/vapi-popup.js
  4. +7 −6 tools/make-safari-meta.py
View
4 platform/safari/Info.plist
@@ -27,12 +27,8 @@
<dict>
<key>Filename</key>
<string>popup.html</string>
- <key>Height</key>
- <real>310</real>
<key>Identifier</key>
<string>popover</string>
- <key>Width</key>
- <real>180</real>
</dict>
</array>
<key>Toolbar Items</key>
View
42 platform/safari/vapi-common.js
@@ -104,48 +104,6 @@ vAPI.i18n = function(s) {
return this.i18nData[s] || s;
};
-/******************************************************************************/
-
-// update popover size to its content
-if (safari.self.identifier === 'popover') {
- var onLoaded = function() {
- // Initial dimensions are set in Info.plist
- var pWidth = safari.self.width;
- var pHeight = safari.self.height;
- var upadteTimer = null;
- var resizePopover = function() {
- if (upadteTimer) {
- return;
- }
-
- upadteTimer = setTimeout(function() {
- safari.self.width = Math.max(pWidth, document.body.clientWidth);
- safari.self.height = Math.max(pHeight, document.body.clientHeight);
- upadteTimer = null;
- }, 20);
- };
-
- var mutObs = window.MutationObserver || window.WebkitMutationObserver;
-
- if (mutObs) {
- (new mutObs(resizePopover)).observe(document, {
- childList: true,
- attributes: true,
- characterData: true,
- subtree: true
- });
- }
- else {
- // Safari doesn't support DOMAttrModified
- document.addEventListener('DOMSubtreeModified', resizePopover);
- }
- };
-
- window.addEventListener('load', onLoaded);
-}
-
-/******************************************************************************/
-
})();
/******************************************************************************/
View
45 platform/safari/vapi-popup.js
@@ -0,0 +1,45 @@
+var whenSizeChanges = function(elm, callback) {
+ var reset = function() {
+ k.style.width = grow.offsetWidth + 10 + "px";
+ k.style.height = grow.offsetHeight + 10 + "px";
+ grow.scrollLeft = grow.scrollWidth;
+ grow.scrollTop = grow.scrollHeight;
+ shrink.scrollLeft = shrink.scrollWidth;
+ shrink.scrollTop = shrink.scrollHeight;
+ w = elm.offsetWidth;
+ h = elm.offsetHeight;
+ }
+ var aux = document.createElement("div");
+ aux.style.cssText = "position:absolute;left:0;top:0;right:0;bottom:0;overflow:scroll;z-index:-1;visibility:hidden";
+ aux.innerHTML = '<div style="position:absolute;left:0;top:0;right:0;bottom:0;overflow:scroll;z-index:-1;visibility:hidden">\
+<div style="position:absolute;left:0;top:0;"></div>\
+</div>\
+<div style="position:absolute;left:0;top:0;right:0;bottom:0;overflow:scroll;z-index:-1;visibility:hidden">\
+<div style="position:absolute;left:0;top:0;width:200%;height:200%"></div>\
+</div>';
+ elm.appendChild(aux);
+ var grow = aux.childNodes[0],
+ k = grow.childNodes[0],
+ shrink = aux.childNodes[1];
+ var w, h;
+ reset();
+ grow.addEventListener("scroll", function() {
+ (elm.offsetWidth > w || elm.offsetHeight > h) && callback();
+ reset();
+ });
+ shrink.addEventListener("scroll", function() {
+ (elm.offsetWidth < w || elm.offsetHeight < h) && callback();
+ reset();
+ });
+};
+var onLoaded = function() {
+ var body = document.body, popover = safari.self;
+ var updateSize = function() {
+ popover.width = body.offsetWidth;
+ popover.height = body.offsetHeight;
+ };
+ updateSize();
+ body.style.position = "relative"; // Necessary for size change detection
+ whenSizeChanges(body, updateSize);
+};
+window.addEventListener('load', onLoaded);
View
13 tools/make-safari-meta.py
@@ -3,6 +3,7 @@
import os
import json
import sys
+import codecs
from time import time
from shutil import rmtree
from collections import OrderedDict
@@ -27,7 +28,7 @@ def mkdirs(path):
for alpha2 in os.listdir(locale_dir):
locale_path = pj(locale_dir, alpha2, 'messages.json')
- with open(locale_path, encoding='utf-8') as f:
+ with codecs.open(locale_path, 'r', encoding='utf8') as f:
string_data = json.load(f, object_pairs_hook=OrderedDict)
if alpha2 == 'en':
@@ -43,7 +44,7 @@ def mkdirs(path):
mkdirs(pj(locale_dir))
- with open(locale_path, 'wt', encoding='utf-8', newline='\n') as f:
+ with codecs.open(locale_path, 'w', encoding='utf8') as f:
json.dump(string_data, f, ensure_ascii=False)
@@ -51,13 +52,13 @@ def mkdirs(path):
proj_dir = pj(os.path.split(os.path.abspath(__file__))[0], '..')
chromium_manifest = pj(proj_dir, 'platform', 'chromium', 'manifest.json')
-with open(chromium_manifest, encoding='utf-8') as m:
+with codecs.open(chromium_manifest, encoding='utf8') as m:
manifest = json.load(m)
manifest['buildNumber'] = int(time())
manifest['description'] = description
-with open(pj(build_dir, 'Info.plist'), 'r+t', encoding='utf-8', newline='\n') as f:
+with codecs.open(pj(build_dir, 'Info.plist'), 'r+', encoding='utf8') as f:
info_plist = f.read()
f.seek(0)
@@ -67,8 +68,8 @@ def mkdirs(path):
update_plist = pj(proj_dir, 'platform', 'safari', 'Update.plist')
update_plist_build = pj(build_dir, '..', os.path.basename(update_plist))
-with open(update_plist_build, 'wt', encoding='utf-8', newline='\n') as f:
- with open(update_plist, encoding='utf-8') as u:
+with codecs.open(update_plist_build, 'w', encoding='utf8') as f:
+ with codecs.open(update_plist, encoding='utf8') as u:
update_plist = u.read()
f.write(update_plist.format(**manifest))
Something went wrong with that request. Please try again.