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 drop image rotate wrong #2322

Merged
merged 2 commits into from
Sep 30, 2018
Merged
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
157 changes: 142 additions & 15 deletions browser/main/lib/dataApi/attachmentManagement.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,118 @@ import i18n from 'browser/lib/i18n'
const STORAGE_FOLDER_PLACEHOLDER = ':storage'
const DESTINATION_FOLDER = 'attachments'
const PATH_SEPARATORS = escapeStringRegexp(path.posix.sep) + escapeStringRegexp(path.win32.sep)
/**
* @description
* Create a Image element to get the real size of image.
* @param {File} file the File object dropped.
* @returns {Promise<Image>} Image element created
*/
function getImage (file) {
return new Promise((resolve) => {
const reader = new FileReader()
const img = new Image()
img.onload = () => resolve(img)
reader.onload = e => {
img.src = e.target.result
}
reader.readAsDataURL(file)
})
}

/**
* @description
* Get the orientation info from iamges's EXIF data.
* case 1: The 0th row is at the visual top of the image, and the 0th column is the visual left-hand side.
* case 2: The 0th row is at the visual top of the image, and the 0th column is the visual right-hand side.
* case 3: The 0th row is at the visual bottom of the image, and the 0th column is the visual right-hand side.
* case 4: The 0th row is at the visual bottom of the image, and the 0th column is the visual left-hand side.
* case 5: The 0th row is the visual left-hand side of the image, and the 0th column is the visual top.
* case 6: The 0th row is the visual right-hand side of the image, and the 0th column is the visual top.
* case 7: The 0th row is the visual right-hand side of the image, and the 0th column is the visual bottom.
* case 8: The 0th row is the visual left-hand side of the image, and the 0th column is the visual bottom.
* Other: reserved
* ref: http://sylvana.net/jpegcrop/exif_orientation.html
* @param {File} file the File object dropped.
* @returns {Promise<Number>} Orientation info
*/
function getOrientation (file) {
const getData = arrayBuffer => {
const view = new DataView(arrayBuffer)

// Not start with SOI(Start of image) Marker return fail value
if (view.getUint16(0, false) !== 0xFFD8) return -2
const length = view.byteLength
let offset = 2
while (offset < length) {
const marker = view.getUint16(offset, false)
offset += 2
// Loop and seed for APP1 Marker
if (marker === 0xFFE1) {
// return fail value if it isn't EXIF data
if (view.getUint32(offset += 2, false) !== 0x45786966) {
return -1
}
// Read TIFF header,
// First 2bytes defines byte align of TIFF data.
// If it is 0x4949="II", it means "Intel" type byte align.
// If it is 0x4d4d="MM", it means "Motorola" type byte align
const little = view.getUint16(offset += 6, false) === 0x4949
offset += view.getUint32(offset + 4, little)
const tags = view.getUint16(offset, little) // Get TAG number
offset += 2
for (let i = 0; i < tags; i++) {
// Loop to find Orientation TAG and return the value
if (view.getUint16(offset + (i * 12), little) === 0x0112) {
return view.getUint16(offset + (i * 12) + 8, little)
}
}
} else if ((marker & 0xFF00) !== 0xFF00) { // If not start with 0xFF, not a Marker
break
} else {
offset += view.getUint16(offset, false)
}
}
return -1
}
return new Promise((resolve) => {
const reader = new FileReader()
reader.onload = event => resolve(getData(event.target.result))
reader.readAsArrayBuffer(file.slice(0, 64 * 1024))
})
}
/**
* @description
* Rotate image file to correct direction.
* Create a canvas and draw the image with correct direction, then export to base64 format.
* @param {*} file the File object dropped.
* @return {String} Base64 encoded image.
*/
function fixRotate (file) {
return Promise.all([getImage(file), getOrientation(file)])
.then(([img, orientation]) => {
const canvas = document.createElement('canvas')
const ctx = canvas.getContext('2d')
if (orientation > 4 && orientation < 9) {
canvas.width = img.height
canvas.height = img.width
} else {
canvas.width = img.width
canvas.height = img.height
}
switch (orientation) {
case 2: ctx.transform(-1, 0, 0, 1, img.width, 0); break
case 3: ctx.transform(-1, 0, 0, -1, img.width, img.height); break
case 4: ctx.transform(1, 0, 0, -1, 0, img.height); break
case 5: ctx.transform(0, 1, 1, 0, 0, 0); break
case 6: ctx.transform(0, 1, -1, 0, img.height, 0); break
case 7: ctx.transform(0, -1, -1, 0, img.height, img.width); break
case 8: ctx.transform(0, -1, 1, 0, 0, img.width); break
default: break
}
ctx.drawImage(img, 0, 0)
return canvas.toDataURL()
})
}

/**
* @description
Expand Down Expand Up @@ -38,26 +150,34 @@ function copyAttachment (sourceFilePath, storageKey, noteKey, useRandomName = tr
}

try {
if (!fs.existsSync(sourceFilePath)) {
reject('source file does not exist')
const isBase64 = typeof sourceFilePath === 'object' && sourceFilePath.type === 'base64'
if (!fs.existsSync(sourceFilePath) && !isBase64) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you reject images that are not base64? e.g. i might drag&drop a pdf. Are all files always base64?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only if the file not exists and the file is not a base64, reject.
In additional, I think you should add return before reject, the code should not run through if the file not exists.

Copy link
Contributor

@ehhc ehhc Aug 22, 2018

Choose a reason for hiding this comment

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

yes, but i mean what happens if you drop a PDF for example. Is that still possible with your changes? Or will it be rejected because it is not bas64?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This patch does nothing for other file. It will work same as before

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. And why don't you use something like this library instead of implementing it yourself? (tbh: first google result) https://www.npmjs.com/package/fix-orientation ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has many dependencies, maybe we don't need to import so many code.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @ehhc. This feature looks good to have. But I think it should not be included in the code base of boostnote. It is almost impossible to maintain...

Copy link
Member

Choose a reason for hiding this comment

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

Why don't we make a npm module for this? And, I think we need some test for this too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll do it some days later.

return reject('source file does not exist')
}

const targetStorage = findStorage.findStorage(storageKey)

const inputFileStream = fs.createReadStream(sourceFilePath)
let destinationName
if (useRandomName) {
destinationName = `${uniqueSlug()}${path.extname(sourceFilePath)}`
destinationName = `${uniqueSlug()}${path.extname(sourceFilePath.sourceFilePath || sourceFilePath)}`
} else {
destinationName = path.basename(sourceFilePath)
destinationName = path.basename(sourceFilePath.sourceFilePath || sourceFilePath)
}
const destinationDir = path.join(targetStorage.path, DESTINATION_FOLDER, noteKey)
createAttachmentDestinationFolder(targetStorage.path, noteKey)
const outputFile = fs.createWriteStream(path.join(destinationDir, destinationName))
inputFileStream.pipe(outputFile)
inputFileStream.on('end', () => {
resolve(destinationName)
})

if (isBase64) {
const base64Data = sourceFilePath.data.replace(/^data:image\/\w+;base64,/, '')
const dataBuffer = new Buffer(base64Data, 'base64')
outputFile.write(dataBuffer, () => {
resolve(destinationName)
})
} else {
const inputFileStream = fs.createReadStream(sourceFilePath)
inputFileStream.pipe(outputFile)
inputFileStream.on('end', () => {
resolve(destinationName)
})
}
} catch (e) {
return reject(e)
}
Expand Down Expand Up @@ -137,10 +257,17 @@ function handleAttachmentDrop (codeEditor, storageKey, noteKey, dropEvent) {
const filePath = file.path
const originalFileName = path.basename(filePath)
const fileType = file['type']

copyAttachment(filePath, storageKey, noteKey).then((fileName) => {
const showPreview = fileType.startsWith('image')
const imageMd = generateAttachmentMarkdown(originalFileName, path.join(STORAGE_FOLDER_PLACEHOLDER, noteKey, fileName), showPreview)
const isImage = fileType.startsWith('image')
let promise
if (isImage) {
promise = fixRotate(file).then(base64data => {
return copyAttachment({type: 'base64', data: base64data, sourceFilePath: filePath}, storageKey, noteKey)
})
} else {
promise = copyAttachment(filePath, storageKey, noteKey)
}
promise.then((fileName) => {
const imageMd = generateAttachmentMarkdown(originalFileName, path.join(STORAGE_FOLDER_PLACEHOLDER, noteKey, fileName), isImage)
codeEditor.insertAttachmentMd(imageMd)
})
}
Expand Down