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

fix!: remove checks for IE and EdgeHTML in core #6336

Merged
merged 7 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions core/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import type {Coordinate} from './utils/coordinate.js';
import * as dom from './utils/dom.js';
import type {Size} from './utils/size.js';
import {Svg} from './utils/svg.js';
import * as userAgent from './utils/useragent.js';


/**
Expand Down Expand Up @@ -256,10 +255,7 @@ export class Comment extends Icon {

/** Show the bubble. Handles deciding if it should be editable or not. */
private createBubble_() {
if (!this.block_.isEditable() || userAgent.IE) {
// MSIE does not support foreignobject; textareas are impossible.
// https://docs.microsoft.com/en-us/openspecs/ie_standards/ms-svg/56e6e04c-7c8c-44dd-8100-bd745ee42034
// Always treat comments in IE as uneditable.
if (!this.block_.isEditable()) {
this.createNonEditableBubble_();
} else {
this.createEditableBubble_();
Expand Down
5 changes: 1 addition & 4 deletions core/contextmenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {Coordinate} from './utils/coordinate.js';
import * as dom from './utils/dom.js';
import {Rect} from './utils/rect.js';
import * as svgMath from './utils/svg_math.js';
import * as userAgent from './utils/useragent.js';
import * as WidgetDiv from './widgetdiv.js';
import {WorkspaceCommentSvg} from './workspace_comment_svg.js';
import type {WorkspaceSvg} from './workspace_svg.js';
Expand Down Expand Up @@ -349,9 +348,7 @@ export function workspaceCommentOption(
}

const wsCommentOption = {
// Foreign objects don't work in IE. Don't let the user create comments
// that they won't be able to edit.
enabled: !userAgent.IE,
enabled: true,
} as ContextMenuOption;
wsCommentOption.text = Msg['ADD_COMMENT'];
wsCommentOption.callback = function() {
Expand Down
7 changes: 2 additions & 5 deletions core/contextmenu_items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import * as eventUtils from './events/utils.js';
import {inputTypes} from './input_types.js';
import {Msg} from './msg.js';
import * as idGenerator from './utils/idgenerator.js';
import * as userAgent from './utils/useragent.js';
import type {WorkspaceSvg} from './workspace_svg.js';


Expand Down Expand Up @@ -362,10 +361,8 @@ export function registerComment() {
},
preconditionFn(scope: Scope) {
const block = scope.block;
// IE doesn't support necessary features for comment editing.
if (!userAgent.IE && !block!.isInFlyout &&
block!.workspace.options.comments && !block!.isCollapsed() &&
block!.isEditable()) {
if (!block!.isInFlyout && block!.workspace.options.comments &&
!block!.isCollapsed() && block!.isEditable()) {
return 'enabled';
}
return 'hidden';
Expand Down
16 changes: 5 additions & 11 deletions core/field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -784,25 +784,19 @@ export abstract class Field implements IASTNodeLocationSvg,
if (!this.borderRect_) {
// Browsers are inconsistent in what they return for a bounding box.
// - Webkit / Blink: fill-box / object bounding box
// - Gecko / Triden / EdgeHTML: stroke-box
// - Gecko: stroke-box
const bBox = (this.sourceBlock_ as BlockSvg).getHeightWidth();
const scale = (this.sourceBlock_.workspace as WorkspaceSvg).scale;
xy = this.getAbsoluteXY_();
scaledWidth = bBox.width * scale;
scaledHeight = bBox.height * scale;
scaledWidth = (bBox.width + 1) * scale;
scaledHeight = (bBox.height + 1) * scale;

if (userAgent.GECKO) {
xy.x += 1.5 * scale;
xy.y += 1.5 * scale;
scaledWidth += 1 * scale;
scaledHeight += 1 * scale;
} else {
if (!userAgent.EDGE && !userAgent.IE) {
xy.x -= 0.5 * scale;
xy.y -= 0.5 * scale;
}
scaledWidth += 1 * scale;
scaledHeight += 1 * scale;
xy.x -= 0.5 * scale;
xy.y -= 0.5 * scale;
}
} else {
const bBox = this.borderRect_.getBoundingClientRect();
Expand Down
7 changes: 0 additions & 7 deletions core/grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ goog.declareModuleId('Blockly.Grid');

import * as dom from './utils/dom.js';
import {Svg} from './utils/svg.js';
import * as userAgent from './utils/useragent.js';
import {GridOptions} from './blockly_options.js';


Expand Down Expand Up @@ -147,12 +146,6 @@ export class Grid {
moveTo(x: number, y: number) {
this.pattern.setAttribute('x', x.toString());
this.pattern.setAttribute('y', y.toString());

if (userAgent.IE || userAgent.EDGE) {
// IE/Edge doesn't notice that the x/y offsets have changed.
// Force an update.
this.update(this.scale_);
}
}

/**
Expand Down
8 changes: 5 additions & 3 deletions core/renderers/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import * as dom from '../../utils/dom.js';
import * as parsing from '../../utils/parsing.js';
import {Svg} from '../../utils/svg.js';
import * as svgPaths from '../../utils/svg_paths.js';
import * as userAgent from '../../utils/useragent.js';


/** An object containing sizing and path information about outside corners. */
Expand Down Expand Up @@ -461,8 +460,11 @@ export class ConstantProvider {

this.START_POINT = svgPaths.moveBy(0, 0);

/** A field's text element's dominant baseline. */
this.FIELD_TEXT_BASELINE_CENTER = !userAgent.IE && !userAgent.EDGE;
/**
* A field's text element's dominant baseline. Pre-2022 this could be false
* for certain browsers.
*/
this.FIELD_TEXT_BASELINE_CENTER = true;

/** A dropdown field's border rect height. */
this.FIELD_DROPDOWN_BORDER_RECT_HEIGHT = this.FIELD_BORDER_RECT_HEIGHT;
Expand Down
20 changes: 15 additions & 5 deletions core/touch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,16 @@ let longPid_: AnyDuringMigration = 0;
export function longStart(e: Event, gesture: Gesture) {
longStop();
// Punt on multitouch events.
if (e instanceof TouchEvent && e.changedTouches &&
e.changedTouches.length !== 1) {
// AnyDuringMigration because: Property 'changedTouches' does not exist on
// type 'Event'.
if ((e as AnyDuringMigration).changedTouches &&
(e as AnyDuringMigration).changedTouches.length !== 1) {
return;
}
longPid_ = setTimeout(function() {
// TODO(#6097): Make types accurate, possibly by refactoring touch handling.
// AnyDuringMigration because: Property 'changedTouches' does not exist on
// type 'Event'.
const typelessEvent = e as AnyDuringMigration;
// Additional check to distinguish between touch events and pointer events
if (typelessEvent.changedTouches) {
Expand Down Expand Up @@ -280,11 +284,17 @@ export function isTouchEvent(e: Event|PseudoEvent): boolean {
*/
export function splitEventByTouches(e: Event): Array<Event|PseudoEvent> {
const events = [];
if (e instanceof TouchEvent) {
for (let i = 0; i < e.changedTouches.length; i++) {
// AnyDuringMigration because: Property 'changedTouches' does not exist on
// type 'PseudoEvent | Event'.
if ((e as AnyDuringMigration).changedTouches) {
// AnyDuringMigration because: Property 'changedTouches' does not exist on
// type 'PseudoEvent | Event'.
for (let i = 0; i < (e as AnyDuringMigration).changedTouches.length; i++) {
const newEvent = {
type: e.type,
changedTouches: [e.changedTouches[i]],
// AnyDuringMigration because: Property 'changedTouches' does not exist
// on type 'PseudoEvent | Event'.
changedTouches: [(e as AnyDuringMigration).changedTouches[i]],
target: e.target,
stopPropagation() {
e.stopPropagation();
Expand Down
7 changes: 1 addition & 6 deletions core/utils/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import * as goog from '../../closure/goog/goog.js';
goog.declareModuleId('Blockly.utils.dom');

import type {Svg} from './svg.js';
import * as userAgent from './useragent.js';


/**
Expand Down Expand Up @@ -269,11 +268,7 @@ export function getTextWidth(textElement: SVGTextElement): number {

// Attempt to compute fetch the width of the SVG text element.
try {
if (userAgent.IE || userAgent.EDGE) {
width = textElement.getBBox().width;
} else {
width = textElement.getComputedTextLength();
}
width = textElement.getComputedTextLength();
} catch (e) {
// In other cases where we fail to get the computed text. Instead, use an
// approximation and do not cache the result. At some later point in time
Expand Down
7 changes: 0 additions & 7 deletions core/utils/svg_math.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import * as deprecation from './deprecation.js';
import {Rect} from './rect.js';
import {Size} from './size.js';
import * as style from './style.js';
import * as userAgent from './useragent.js';


/**
Expand Down Expand Up @@ -196,12 +195,6 @@ export function getViewportBBox(): Rect {
export function getDocumentScroll(): Coordinate {
const el = document.documentElement;
const win = window;
if (userAgent.IE && win.pageYOffset !== el.scrollTop) {
// The keyboard on IE10 touch devices shifts the page using the pageYOffset
// without modifying scrollTop. For this case, we want the body scroll
// offsets.
return new Coordinate(el.scrollLeft, el.scrollTop);
}
return new Coordinate(
win.pageXOffset || el.scrollLeft, win.pageYOffset || el.scrollTop);
}
Expand Down
13 changes: 2 additions & 11 deletions core/utils/toolbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import type {ConnectionState} from '../serialization/blocks.js';
import type {CssConfig as CategoryCssConfig} from '../toolbox/category.js';
import type {CssConfig as SeparatorCssConfig} from '../toolbox/separator.js';
import * as Xml from '../xml.js';
import * as userAgent from './useragent.js';


/**
Expand Down Expand Up @@ -405,16 +404,8 @@ function addAttributes(node: Node, obj: AnyDuringMigration) {
export function parseToolboxTree(toolboxDef: Element|null|string): Element|
null {
if (toolboxDef) {
if (typeof toolboxDef !== 'string') {
if (userAgent.IE && toolboxDef.outerHTML) {
// In this case the tree will not have been properly built by the
// browser. The HTML will be contained in the element, but it will
// not have the proper DOM structure since the browser doesn't support
// XSLTProcessor (XML -> HTML).
toolboxDef = toolboxDef.outerHTML;
} else if (!(toolboxDef instanceof Element)) {
toolboxDef = null;
}
if (typeof toolboxDef !== 'string' && !(toolboxDef instanceof Element)) {
toolboxDef = null;
}
if (typeof toolboxDef === 'string') {
toolboxDef = Xml.textToDom(toolboxDef);
Expand Down
19 changes: 4 additions & 15 deletions core/utils/useragent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ goog.declareModuleId('Blockly.utils.userAgent');
/** The raw useragent string. */
let rawUserAgent: string;

let isIe: boolean;

let isEdge: boolean;

let isJavaFx: boolean;

let isChrome: boolean;
Expand Down Expand Up @@ -63,18 +59,16 @@ function has(name: string): boolean {

// Browsers. Logic from:
// https://github.com/google/closure-library/blob/master/closure/goog/labs/useragent/browser.js
isIe = has('Trident') || has('MSIE');
isEdge = has('Edge');
// Useragent for JavaFX:
// Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.44
// (KHTML, like Gecko) JavaFX/8.0 Safari/537.44
isJavaFx = has('JavaFX');
isChrome = (has('Chrome') || has('CriOS')) && !isEdge;
isChrome = (has('Chrome') || has('CriOS'));
Copy link
Contributor

Choose a reason for hiding this comment

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

do we use these for anything? maybe we should fully get rid of the ones we don't need at all


// Engines. Logic from:
// https://github.com/google/closure-library/blob/master/closure/goog/labs/useragent/engine.js
isWebKit = has('WebKit') && !isEdge;
isGecko = has('Gecko') && !isWebKit && !isIe && !isEdge;
isWebKit = has('WebKit');
isGecko = has('Gecko') && !isWebKit;

// Platforms. Logic from:
// https://github.com/google/closure-library/blob/master/closure/goog/labs/useragent/platform.js
Expand All @@ -97,12 +91,6 @@ isMobile = !isTablet && (isIPod || isIPhone || isAndroid || has('IEMobile'));
/** @alias Blockly.utils.userAgent.raw */
export const raw: string = rawUserAgent;

/** @alias Blockly.utils.userAgent.IE */
export const IE: boolean = isIe;

/** @alias Blockly.utils.userAgent.EDGE */
export const EDGE: boolean = isEdge;

/** @alias Blockly.utils.userAgent.JavaFx */
export const JavaFx: boolean = isJavaFx;

Expand All @@ -111,6 +99,7 @@ export const CHROME: boolean = isChrome;

/** @alias Blockly.utils.userAgent.WEBKIT */
export const WEBKIT: boolean = isWebKit;

/** @alias Blockly.utils.userAgent.GECKO */
export const GECKO: boolean = isGecko;

Expand Down
10 changes: 0 additions & 10 deletions tests/mocha/contextmenu_items_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -356,16 +356,6 @@ suite('Context Menu Items', function() {
chai.assert.equal(this.commentOption.preconditionFn(this.scope), 'enabled');
});

test.skip('Hidden for IE', function() {
const oldState = Blockly.utils.userAgent.IE;
try {
Blockly.utils.userAgent.IE = true;
chai.assert.equal(this.commentOption.preconditionFn(this.scope), 'hidden');
} finally {
Blockly.utils.userAgent.IE = oldState;
}
});

test('Hidden for collapsed block', function() {
// Must render block to collapse it properly.
this.block.initSvg();
Expand Down